For Panda, I made a serial COM port test. It works as conceived, what is striking me that the Garbage Collector (GC) is doing more work than I expected.
Most buffers are declared static, as the remaining static to dynamic buffers are changed, this has little effect on the GC
A question about the source of the test program, the code as shown is optimum for NETMF?
Whether there are better constructions possible?
the main:
using System.Threading;
using System.IO.Ports;
using System.Text;
using Microsoft.SPOT;
using Microsoft.SPOT.Hardware;
using GHIElectronics.NETMF.FEZ;
namespace hint
{
public class Program
{
public static void Main()
{
new Thread(() => flashOnboardLed(200)).Start();
serialIO rt = new serialIO("COM1", 57600);
int msgNbr = 0;
string msg;
int cnt = 0;
while (true)
{
if (rt.recvReady())
{
msg = rt.recvMsg();
if (msg != null)
Debug.Print(msg);
}
if ((cnt > 1) && (!rt.recvBusy()))
{
msgNbr++;
rt.sendMsg("Message number: " + msgNbr.ToString() + "\r\n");
cnt = 0;
}
cnt++;
Thread.Sleep(10);
}
}
public static void flashOnboardLed(object flashTime)
{
OutputPort led = new OutputPort((Cpu.Pin)FEZ_Pin.Digital.LED, false);
while (true)
{
led.Write(!led.Read());
Thread.Sleep((int)flashTime);
}
}
}
}
the class:
using System;
using System.IO.Ports;
using System.Text;
using Microsoft.SPOT;
namespace hint
{
public class serialIO
{
static SerialPort UART;
const int maxBuf = 127;
static byte[] _txBuf = new byte[maxBuf];
static byte[] _rxBuf = new byte[maxBuf];
static int _rxInx;
static bool _rxReady = false;
static string _port;
public serialIO(string port, int baudrate)
{
UART = new SerialPort(port, baudrate);
UART.ReadTimeout = 0;
UART.ErrorReceived += new SerialErrorReceivedEventHandler(UART_ErrorReceived);
UART.Open();
UART.DataReceived += new SerialDataReceivedEventHandler(UART_DataReceived);
_port = port;
_rxInx = 0;
}
public bool recvBusy()
{
if ((_rxInx > 0) || (UART.BytesToRead > 0))
return true;
else
return false;
}
public bool recvReady()
{
return _rxReady;
}
public void sendMsg(string msg)
{
_txBuf = Encoding.UTF8.GetBytes(msg);
UART.Write(_txBuf, 0, _txBuf.Length);
}
public string recvMsg()
{
if (_rxReady)
{
char[] chars = Encoding.UTF8.GetChars(_rxBuf);
string str = new string(chars);
for (int k = 0; k < _rxInx; k++)
_rxBuf[k] = 0;
_rxInx = 0;
_rxReady = false;
return str;
}
else
return null;
}
static void UART_DataReceived(object sender, SerialDataReceivedEventArgs e)
{
int rxCnt;
byte[] buf = new byte[UART.BytesToRead];
// if (!_rxReady)
{
rxCnt = UART.Read(buf, 0, buf.Length);
UART.Write(buf, 0, buf.Length);
for (int i = 0; i < rxCnt; i++)
{
if ((buf[i] != 13) && (buf[i] != 10) && (_rxInx < maxBuf))
{
_rxBuf[_rxInx++] = buf[i];
}
else
{
if ((buf[i] == 13) || (_rxInx == maxBuf))
{
_rxReady = true;
break;
}
}
}
}
}
static void UART_ErrorReceived(object sender, SerialErrorReceivedEventArgs e)
{
Debug.Print(_port + " receive error: " + e.EventType.ToString());
}
}
}
For what its worth. Anytime your doing a spinwait like that and/or checking BytesReceived, is clue your pattern needs to change. Here we are blocking 10ms even if we just received many bytes. And I wish they would just remove BytesReceived, it leads people down the wrong road. If we block, we should block waiting for actual bytes. Such as:
using System.Threading;
using GHIElectronics.NETMF.USBClient;
using Microsoft.SPOT;
namespace PandaSimpleSerial
{
public class Program
{
public static void Main()
{
USBC_CDC port = InitCDC();
string line = null;
while ((line=port.ReadLine()) != "EOF")
{
port.WriteLine(line); // Echo back line.
Debug.Print("Panda wrote: " + line);
}
Debug.Print("End of Program");
}
private static USBC_CDC InitCDC()
{
Debug.Print("Hit Break All in VS to reattach debugger to port, then F5/Continue.");
USBC_CDC sPort = USBClientController.StandardDevices.StartCDC_WithDebugging();
sPort.ReadTimeout = -1; // Block waiting for chars.
while (true)
{
if (USBClientController.GetState() != USBClientController.State.Running)
{
Debug.Print("Waiting to connect.");
Thread.Sleep(500);
}
else
{
Debug.Print("CDC comm port connected: ");
break;
}
}
return sPort;
}
}
}
using System;
using Microsoft.SPOT;
using System.IO.Ports;
using System.Text;
using GHIElectronics.NETMF.USBClient;
namespace System.Runtime.CompilerServices
{
[AttributeUsage(AttributeTargets.Assembly | AttributeTargets.Class | AttributeTargets.Method)]
public sealed class ExtensionAttribute : Attribute { }
}
namespace PandaSimpleSerial
{
public static class Extentions
{
public static int Write(this USBC_CDC port, string value)
{
byte[] buf = Encoding.UTF8.GetBytes(value);
return port.Write(buf, 0, buf.Length);
}
public static int WriteLine(this USBC_CDC port, string value)
{
byte[] buf = Encoding.UTF8.GetBytes(value+"\r\n");
return port.Write(buf, 0, buf.Length);
}
/// <summary>
/// Reads port until LineFeed char (\n = (char)10)
/// If line exceeds maxBufSize, exception will be thrown.
/// </summary>
public static string ReadLine(this USBC_CDC port, bool trimNewLine=true, int maxBufSize = 300)
{
if (maxBufSize <= 0) throw new ArgumentOutOfRangeException("maxBufSize");
byte[] buf = new byte[maxBufSize];
int curPos = 0;
while (curPos < maxBufSize)
{
int read = port.Read(buf, curPos, 1);
if (read == 0) break;
if (buf[curPos] == 10) break;
curPos += read;
}
if (curPos == 0) return null;
if (buf[curPos] != 10)
throw new InvalidOperationException("Line size > maxBufSize.");
if (trimNewLine)
{
int removeCount = 1;
if (curPos >= 2 && buf[curPos - 1] == 13)
removeCount = 2;
int count = curPos - removeCount + 1;
byte[] tmp = new byte[count];
Array.Copy(buf, tmp, count);
buf = tmp;
}
char[] ca = Encoding.UTF8.GetChars(buf);
return new string(ca);
}
}
}
If you want to save a buffer copy, set ReadLine(trimNewLine=false) and just deal with the newline chars at the end. If you just echoing them or printing them, then no need to remove them anyway. Also set maxBufSize to largest line you expect and no more. Adjust for UTF8 chars as needed.
Don’t be overly concerned about the GC unless you see it becomes an issue. The GC can manage memory better then we can manually and does it in native code.
Reuse buffers only if you have fixed message blocks. Even then, it is discouraged. Zeroing buffers is slower in managed code then having the GC alloc and free.
Use Len prepended messages over delimited records (i.e. new line chars) if possible. Knowing the size we are reading before hand allows more efficient buffer allocation and is “saner”. Unless you need to interface with some existing TTY on other side, use Len-Prefix records. Variable len delimited records leads us down the road of allocating more buffer space then we need and/or doing a copy to shink the buffer to final size.
@ William, I need your recommendations as far as possible included in my test program. The program is now in my opinion better.
Your example program with the CDC unfortunately I can not use because I like the Panda wirelessly connect to a PC using a Bluetooth connection. In the main loop of the test program, I later include the processing of sensor results add. That is why periodically wondered whether some new assignment messages.
using System.Threading;
using System.IO.Ports;
using System.Text;
using Microsoft.SPOT;
using Microsoft.SPOT.Hardware;
using GHIElectronics.NETMF.FEZ;
namespace hints
{
public class Program
{
public static void Main()
{
new Thread(() => flashOnboardLed(200)).Start();
serialIO rt = new serialIO("COM1", 57600);
int msgNbr = 0;
string msg;
int cnt = 0;
const int recvInterval = 50;
const int sendInterval = 200/recvInterval;
while (true)
{
//
//
//
if (rt.recvReady())
{
msg = rt.recvMsg();
if (msg != null)
Debug.Print(msg);
}
if (cnt > (sendInterval))
{
msgNbr++;
rt.sendMsg("Message number: " + msgNbr.ToString() + "\r\n");
cnt = 0;
}
cnt++;
Thread.Sleep(recvInterval);
}
}
public static void flashOnboardLed(object flashTime)
{
OutputPort led = new OutputPort((Cpu.Pin)FEZ_Pin.Digital.LED, false);
while (true)
{
led.Write(!led.Read());
Thread.Sleep((int)flashTime);
}
}
}
}
using System.Threading;
using System;
using System.IO.Ports;
using System.Text;
using Microsoft.SPOT;
namespace hints
{
public class serialIO
{
static private SerialPort UART;
static private bool _rxReady = false;
static private string _port;
static private string _rxStr = null;
public serialIO(string port, int baudrate)
{
UART = new SerialPort(port, baudrate);
UART.ReadTimeout = 0;
UART.ErrorReceived += new SerialErrorReceivedEventHandler(UART_ErrorReceived);
UART.Open();
new Thread(() => UART_DataReceive()).Start();
_port = port;
}
public bool recvReady()
{
return _rxReady;
}
public string recvMsg()
{
if (_rxReady)
{
_rxReady = false;
return _rxStr;
}
else
return null;
}
public int sendMsg(string msg)
{
byte[] buf = Encoding.UTF8.GetBytes(msg);
return UART.Write(buf, 0, buf.Length);
}
static void UART_DataReceive()
{
const int maxBuf = 64;
byte[] _rxBuf = new byte[maxBuf];
int _rxInx = 0;
bool _rxRecv = false;
while (true)
{
if (UART.BytesToRead > 0)
{
byte[] buf = new byte[1];
if (!_rxReady)
{
int cnt = UART.Read(buf, 0, 1);
}
if ((buf[0] < 0x20) || (buf[0] > 0x7E))
{
continue;
}
if (buf[0] == '[') // start text delimiter
{
_rxInx = 0;
Array.Clear(_rxBuf, _rxInx, maxBuf);
_rxRecv = true;
continue;
}
if (buf[0] == ']') // stop text delimiter
{
if (_rxReady)
{
throw new InvalidOperationException(_port + ": throughput error.");
}
else
{
char[] chars = Encoding.UTF8.GetChars(_rxBuf);
_rxStr = new string(chars);
_rxInx = 0;
Array.Clear(_rxBuf, _rxInx, maxBuf);
_rxRecv = false;
_rxReady = true;
}
continue;
}
if (_rxRecv)
{
_rxBuf[_rxInx] = buf[0];
if (_rxInx < maxBuf - 1)
{
_rxInx++;
}
}
}
else
{
Thread.Sleep(50);
}
}
}
static void UART_ErrorReceived(object sender, SerialErrorReceivedEventArgs e)
{
Debug.Print(_port + " receive error: " + e.EventType.ToString());
// throw new InvalidOperationException(_port + ": receive error: " + e.EventType.ToString());
}
}
}
The CDC port was just example, you can drop-in replace with SerialPort or other. Not sure what you tell you. Your still using Sleep and BytesToRead. Did you actually read my post? The hardest leason to learn (for me), is don’t fall in love with a bad design and try to fix it with more design. Forget about BytesToRead. Pretend it does not exist. And don’t sleep in a loop. Your sleeping 90%+ of the time, when you should be reading.
Also. Clearing byte[] smells funny. In general, this is error prone and actually slower then just dispose (fall out) and alloc a new array. Let the gc do what it is good at. Also use Len pre-pended records instead of two delims. It works much better and your not guessing on buffer sizes needed and you can read/demand in full. More efficient reads.
Hi Wiliam, i’d love your take on a serial comms “challenge” I have. You have your mind around this much better than I could ever imagine !
I am connecting to a serial device that I can’t control (it’s a commercial product and I’m just one of thousands of owners; so I can’t control it’s data responses). It has a simple command protocol - send it a ASCII command, it responds. There are SET type commands but mainly there are READ commands. I am going to write a datalogger - it will basically read info and then use ethernet to do a web-service POST.
There are two types of responses. Single line responses start with response text, and end in a \r. Multi-line start and end with a double-quote and \r combo. All of the responses are ASCII text and all of them vary in length, even from query-to-query (since the returned value is a measurement that changes) - but the responses are in the tens of characters, not hundreds. Most, not all, commands and responses I intend to use are single line, so I want to handle both.
The challenge I am thinking (I’m not good at jumping to code ) is about transmission failures etc; not hanging the app waiting for a terminator that was failed to be recieved for instance (or when nothing comes back since the serial line was disconnected or something).
Simplistically I can read data as it arrives in a datarecieved event handler and push into a buffer, and sleep for an acceptable transmission time with plenty of leeway and then check if i have a whole reply; discard if not, or interpret if so; The only thing I have against that is that the sleep time is wasted especially on some of the very short queries.
Can you think of a different pattern that would better optimise the processor instead of this kind of wait?
@ Brett. I don’t think I am clear on your systems end of line protocol. Normally, end of line is “\r\n” or just “\n”. Put if so, then something like this (not tested):
private static string SendReceive(SerialPort port, string request)
{
port.Write(request);
string line = port.ReadLine(port, 13, 300); // \r is line seperator.
// if line starts with ", then keep reading lines until line that ends with ".
return line;
}
public static string ReadLine(SerialPort port, int delim = 10, int maxBufSize = 300)
{
//CR 13 LF 10 = "\r\n"
if (maxBufSize <= 0) throw new ArgumentOutOfRangeException("maxBufSize");
byte[] buf = new byte[maxBufSize];
int curPos = 0;
while (curPos < maxBufSize)
{
int read = port.Read(buf, curPos, 1);
if (read == 0) break;
if (buf[curPos] == delim) break;
curPos += read;
}
// curPos will be zero if first read is zero or first read is delim byte.
if (curPos == 0) return null;
if (buf[curPos] != delim)
throw new InvalidOperationException("Line size > maxBufSize.");
char[] ca = Encoding.UTF8.GetChars(buf);
return new string(ca);
}
You can change end of line delim to either 10 or 13. From what I see in your protocol (is this Cisco router?), a line in all cases ends with \r. You just need to add code to process your quote multi-line thing. If line starts with ", then keep reading lines until you read a line that ends with ". Then you have the full reply. Process next request, and repeat. If this is the main point of your device, then I might just keep it in the main thread loop. Other things (LEDs, etc) I might add seperate threads to handle those as seperate tasks.
In terms of timeout, that always depends. Set a worst case timeout. Make it larger then the max delay you should ever reasonably see. Question always is, what to do when you get a timeout? Log error and just quit? Start some kind of retry logic? There is no best answer there as depends on your situation.
@ William, You’re right that I am not your recommendation was accepted in full.
I kept my old solution too long. I have to get used to the limited functions of netmf. In a C # Windows application I used the following code to a delimited text to read:
string str = serialPort1.ReadTo ("\ x2");
str = serialPort1.ReadTo str = ("\ x3");
I have further modified the test program, the class serialIO replaced by a thread. It now seems a good basis for further develop.
The frequency of calling the GC appears to be reduced.
But it puzzled me that with such a small program, already one quarter of available memory.
See the GC report:
GC: 3msec 18468 bytes used, 45912 bytes available
Type 0F (STRING ): 180 bytes
Type 11 (CLASS ): 1452 bytes
Type 12 (VALUETYPE ): 132 bytes
Type 13 (SZARRAY ): 456 bytes
Type 15 (FREEBLOCK ): 45912 bytes
Type 17 (ASSEMBLY ): 9336 bytes
Type 18 (WEAKCLASS ): 48 bytes
Type 19 (REFLECTION ): 24 bytes
Type 1B (DELEGATE_HEAD ): 216 bytes
Type 1D (OBJECT_TO_EVENT ): 120 bytes
Type 1E (BINARY_BLOB_HEAD ): 156 bytes
Type 1F (THREAD ): 1920 bytes
Type 20 (SUBTHREAD ): 240 bytes
Type 21 (STACK_FRAME ): 2904 bytes
Type 27 (FINALIZER_HEAD ): 144 bytes
Type 31 (IO_PORT ): 144 bytes
Type 33 (I2C_XACTION ): 48 bytes
Type 34 (APPDOMAIN_HEAD ): 72 bytes
Type 36 (APPDOMAIN_ASSEMBLY ): 876 bytes
Should I be concerned about the available memory of the Panda for a robot driver?
no you have plenty of memory. Assembly space seems to be the main thing which has to do with what assemblies you added. they don’t need to load multiple times and only take up a certain amount of space. Just remember to think small.
If you want ReadTo like function, you could roll it yourself. The reason we ask for bufSize is because we don’t want to dynamically expand and copy buffers. If wanted dynamic, can add expanding buffer and drop the maxBufSize parm.
public static string ReadTo(this SerialPort port, string pattern, int maxBufSize = 300)
{
if (port == null || !port.IsOpen)
throw new ArgumentException("Port null or closed");
if (pattern == null)
throw new ArgumentNullException("pattern");
byte[] patternBuf = Encoding.UTF8.GetBytes(pattern);
if (maxBufSize <= 0) throw new ArgumentOutOfRangeException("maxBufSize");
byte[] buf = new byte[maxBufSize];
int curPos = 0;
bool readPattern = false;
while (curPos < maxBufSize)
{
int read = port.Read(buf, curPos, 1);
if (read == 0)
throw new InvalidOperationException("Read zero.");
if (curPos >= patternBuf.Length - 1 && buf.EndsWith(curPos, patternBuf))
{
readPattern = true;
// Clear pattern bytes from buf.
int num = patternBuf.Length;
int index = curPos;
while (num-- > 0)
{
buf[index--] = 0;
}
break;
}
curPos += read;
}
if (!readPattern)
throw new InvalidOperationException("Line size > maxBufSize.");
char[] ca = Encoding.UTF8.GetChars(buf);
return new string(ca);
}
public static bool EndsWith(this byte[] buf, int index, byte[] pattern)
{
if (index >= buf.Length || index < 0) throw new ArgumentOutOfRangeException("index");
if (buf.Length == 0 && pattern.Length == 0) return true;
if (pattern.Length - 1 > index) return false;
int patIndex = pattern.Length - 1;
while (patIndex >= 0)
{
if (buf[index--] != pattern[patIndex--]) return false;
}
return true;
}
) is about transmission failures etc; not hanging the app waiting for a terminator that was failed to be recieved for instance (or when nothing comes back since the serial line was disconnected or something).