lock shouldn’t be an issue,thread function doesn’t depend on that and it should signal the completed event back.
In that case, I think you want this to be checking for null instead of string.Empty…
while ((Line = SR.ReadLine()) != string.Empty)
Good catch! I missed that in the original code.
Also make sure to use William’s fixed ReadLineEx function from this thread. MF ReadLine doesn’t return null as it should (it is a known bug).
http://www.tinyclr.com/forum/13/1620/
private ArrayList ReadFromLog()
{
if (File.Exists(LogPath))
{
LogItems = new ArrayList();
using (StreamReader SR = new StreamReader(LogPath))
{
string Line;
while ((Line = ReadLineEx(SR)) != null)
{
LogItems.Add(Line);
}
}
}
return LogItems;
}
Wow.
From William’s fix, it looks like that problem might only happen if the last line in the file doesn’t have a trailing \n on it. I wonder if people reading “” at the end is always the result of using Write instead of WriteLine or if there are other cases that can make it happen.
This is probably all great stuff and the correct way of doing this logger function, but for me it looks way to complicated. As a hobbyist an fairly new to C# Im all for the KISS (Keep It Simple Stupid) approach. I must admit that this doesnt look FEZ to me at all. 
Besides getting complicated, the read still locks up the cobra. I think I will move back to my initial code and reduce the threads to one.
Where complicated? Version I posted is like 8 lines of code. I don’t count the blockingqueue which is reusable library code and a thread safe code is must for almost anything consumer/producer.
William,
I will give your code a try in the morning and report back.
Sounds good Eric. You should be able to add your writer pretty easy. Let me know. I am sure using a new thread for each write is not good way, and probably will hide other race issues. Threading is hard - damn hard. It will never be a beginner field. When we jump into that area, we need to use abstractions (i.e. blockingqueue) and well-known patterns that codifies most of hard work and is reusable.
Eric,
If you want to learn more about different design patters including “Blocking queue”. I recommend this article:
William,
in your blockingqueue class there’s a reference to MF.Threading for Monitor2, where’s that coming from?
It is on Fezzer
Thanks Architect, I found it.
I tried William’s code but that throws a very informative error:
------ Rebuild All started: Project: FEZ Cobra SD Card, Configuration: Debug Any CPU ------
MMP : error MMP0000: CLR_E_FAIL
========== Rebuild All: 0 succeeded, 1 failed, 0 skipped ==========
Any idea?
Did you resolve it? I haven’t seen that error before.
No, I didn’t found a solution yet.
I have seen it, but now I can’t remember exactly. I am thinking:
- Firmware issue.
- Bad assembly reference (i.e. old or not netmf). Check no ref to non-target Fez board.
I build new Panda project and pulled down Monitor2 and BlockingQueue from fezzer. Works on Panda and Domino. Guess could also (but don’t think so) be something with cobra, but I don’t have to test.
I see attempts to keep this threaded, but why not use a thread safe singleton for logging?
This is not tested (written in notepad quickly), but something like this should serve as a pretty good starting place for logging.
public class Logger
{
private static PersistentStorage sdPS;
private static System.IO.StreamWriter Output = null;
private static Logger logger = null;
private static Object lockObject = typeof(Logger);
public static string LogFile = @ "\SD\log.txt";
public static int LogLevel = 1;
private Logger()
{
sdPS = new PersistentStorage("SD");
sdPS.MountFileSystem();
}
public static Logger Instance()
{
//lock object to make it thread safe
lock (lockObject)
{
if (logger == null)
{
logger = new Logger();
}
}
return logger;
}
public static void WriteLog(string s)
{
try
{
if (Output == null)
{
Output = new System.IO.StreamWriter(LogFile, true, System.Text.UnicodeEncoding.Default);
}
Output.WriteLine(System.DateTime.Now + "," + "0" + "," + s);
CloseLog();
}
catch (Exception ex)
{
Console.WriteLine(ex.Message, new object[0]);
}
}
public static void WriteLog(string s, int severity)
{
try
{
if (severity <= LogLevel)
{
if (Output == null)
{
Output = new System.IO.StreamWriter(LogFile, true, System.Text.UnicodeEncoding.Default);
}
Output.WriteLine(System.DateTime.Now + " | " + severity + " | " + s);
CloseLog();
}
}
catch (Exception ex)
{
Console.WriteLine(ex.Message, new object[0]);
}
}
public static void CloseLog()
{
try
{
if (Output != null)
{
Output.Close();
Output = null;
}
}
catch (Exception ex)
{
Console.WriteLine(ex.Message, new object[0]);
}
}
}
You might want to consider using a static constructor to instaniate the singleton Logger object.
Would you not want a lock for writes? Without a lock, two threads could be writting to the log file at the same time from different buffers.
For the locking object, could you not use the singleton Logger object?
Might want to make the Instance method a property?
Previous code was a collage of old code i threw into notepad without really checking.
public sealed class Logger
{
public delegate void LogMessageHandler(DateTime Timestamp, string Message);
public event LogMessageHandler MessageLogged;
private static object StreamLock = new object();
private static readonly Logger instance = new Logger();
public static Logger Instance { get { return instance; } }
public static string LogFile;
public static bool UseRunClock = true;
static Logger() { }
private Logger()
{
sdPS = new PersistentStorage("SD");
sdPS.MountFileSystem();
if (LogFile == null || LogFile == "") { LogFile = @ "\SD\log.txt"; }
}
~Logger()
{
MessageLogged = null;
if (sdPS != null)
{
sdPS.UnmountFileSystem();
sdPS.Dispose();
}
}
private PersistentStorage sdPS;
private System.IO.StreamWriter OutputStream = null;
public void Write(string Message)
{
// Deadlocks the current write operation.
lock (StreamLock)
{
try
{
if (!File.Exists(LogFile)) { File.Create(LogFile); }
if (OutputStream == null) { OutputStream = new System.IO.StreamWriter(LogFile, true); }
DateTime Timestamp = GetTimestamp();
OutputStream.WriteLine(Timestamp.ToUniversalTime().ToString() + "," + Message);
if (MessageLogged != null)
MessageLogged(Timestamp, Message);
}
catch
{
// Cant really do anything if the write operation fails now can we...
}
}
}
public void Clear()
{
lock (StreamLock)
{
try
{
if (File.Exists(LogFile)) { File.Delete(LogFile); }
}
catch
{
// cant log because we're evidently trying to delete the logs.
}
}
}
private static DateTime GetTimestamp()
{
if (UseRunClock)
return GHIElectronics.NETMF.Hardware.RealTimeClock.GetTime();
else
return DateTime.Now;
}
}
Usage can be accomplished from pretty much anywhere in the code. This singleton will attempt to lock on anything that would touch the output stream.
Logger logger = Logger.Instance;
logger.Write("Write to log");
// or
Logger.Instance.Write("Blah blah blah");