This seems to take care of the log data pretty much entirely and is
possibly a much simpler alternative solution to the previous two
commits. However, for full robustness, I think it makes sense to keep
all three of these changes together. Also, it's entirely possible that
this approach introduces a performance regression: I haven't
particularly looked at how the buffer flushing is implemented, but if
it ends up doing looped reads with a timeout, this could slow down
command processing for the SPID object significantly. Since I've only
tested this through the command line interface, I have not taken a
close look at performance.
When the settings are saved via the front panel on the MD-01, the
following debug messages are printed on COM 0:
thread_motionController: settings changed!\r\n
thread_protocols: settings changed!\r\n
Notably, because these aren't timestamped the way the other debug
messages are, they were missing the our debug message sieve and causing
protocol errors. Address this by treating anything that doesn't start
with the ROT2PROG start byte ('W') as a log frame.
I have an MD-01 controller running firmware 2.0.237 that is connected
to the computer via the COM0 DB9 port. When it receives a control
command to move the rotator, it prints debug logs to the serial output
that look like this:
input:
W3600\x0A4500\x0A\x2F\x20
output:
W\x03\x06\x00\x00\x0A\x04\x05\x01\x00\x0A\x20
287925671: in motion\r\n
287925673: Change motion state M0 to mcsStart\r\n
287925678: GO A 0.000000 0.000000 t ---\r\n
287925680: in motion\r\n
287925683: Change motion state M1 to mcsStart\r\n
287925686: GO E 90.000000 91.000000 t ---\r\n
287925690: distance to go M0 too small. End.\r\n
287925694: Change motion state M0 to mcsStopped\r\n
287925698: Stop on motor 0 on angle 0.000000\r\n
287925703: Change motion state M1 to mcsRunning\r\n
287926610: distance to go M1 too small. End.\r\n
287926613: Change motion state M1 to mcsStopped\r\n
287926617: Stop on motor 1 on angle 90.000000\r\n
Note that the response frame is not necessarily present in an specific
order relative to the log messages: I have seen it come after the logs
as well. Because the current implementation just slurps up response
bytes without checking the framing or anything, as soon as any of this
log data enters the command stream, all subsequent commands will read
completely bogus responses.
Regardless of whether it's due to a misconfiguration, a weird artifact
of the firmware, or something more sinister, the MD-01 is awkwardly
interspersing its normal fixed-size-frame response with these
line-based log messages. As shown above, the log messages appear to be
consistently of the format <timestamp>: <message>\r\n, where
<timestamp> is some kind of integer timestamp (possibly relative to
unit boot) and <message> is an ASCII string.
Due to poor(?) design decisions by the protocol designers, the frame
start and end bytes are both printable ASCII characters ('W' and ' '
respectively) and the MD-01 response frame contains no other
validation information (such as a CRC), which means that a valid log
line could fairly easily contain a character sequence that is
indistinguishable from a valid response frame, without actually being
a valid response frame.
However, since the log messages appear to be reasonably strictly
structured, we can make a small number of assumptions that will allow
us to reliably separate response frames from log lines without having
to fall back on a heuristic-based parsing strategy. These assumptions
are as follows:
1. A log line will always begin with an ASCII character in the range
[0-9], none of which are the frame start byte.
2. A log line will never contain \r\n in the middle of the line (i.e.
multi-line log messages do not exist). This means a log "frame" will
always be of the form [0-9]<anything>\r\n.
3. The controller will not emit a response frame in the middle of a log
line.
4. The operating system's serial port read buffer is large enough that
we won't lose data while accumulating log messages between commands.
Provided the above assumptions are true, a simple state machine can be
used to parse the response by treating the log lines as a different
type of frame. This could be made much more robust by applying
additional heuristics for specific packets (e.g. get_position has some
reasonably strict numerical bounds that could be used to sanity check
the contents of the reply frame).
This is necessary to keep alignment between the client view of rig->state and the shared libarary view
Including PTHREAD functions in rig->state necessitated this as HAVE_PTHREAD only defined during hamlib compilation.
Clients including hamlib/rig.h did not pick up all the HAVE* config variable that control what gets included
https://github.com/Hamlib/Hamlib/issues/947
George Baltz N3GB