FocusSerial plugin doesn't seem talkative

I’m on Linux and using Model01-Firmware v1.22-49-gf3f687ed63e7 (current git master).

I can connect to /dev/ttyACM0 with several different programs, but it’s behaving as if every keypress I send is a newline - adding the FocusTestCommand example to the sketch works, but trying to send the string “test” over serial just gives me four “.” replies, spaced 2 lines apart.

Looking at the source I can’t understand why it’s doing that; it should be waiting for me to press enter, right? It feels like I might be overlooking something obvious.

I’m not really sure why this happens either, but I can reproduce the problem with screen and others too. I’ve been using the bin/focus-test script included with Kaleidoscope:

$ bin/focus-test help
help
version
keymap.map
keymap.roLayers
palette
colormap.map
settings.defaultLayer
settings.valid?
settings.version
settings.crc
eeprom.contents
eeprom.free
hostos.type
cycletime

Ah, I’m getting somewhere now, it works fine through that. In fact I can open a window and echo test > /dev/ttyACM0 and the result’s printed in a serial terminal no problem.

Now I see what’s going on: typing only sends one char per write(), but the FocusSerial read loop expects the input buffer to contain an entire line and tries to call Serial.read() on an empty input, filling the rest of the buffer with -1.

I tried hacking it by moving the loop counter to a class-static, and got it to kinda work: typing help[space] in a serial terminal prints a response now. I definitely don’t trust myself with this kind of low-level code though, and there’s not much of a use case for interactively typing to one’s keyboard. It might be better to just document the weirdness and tell people to always use line buffering.

2 Likes

If we can make it work interactively, I’d love that. Can you share the code you have now? I’d love to take it further.

Here’s a diff, caveat emptor. It won’t handle typed in newlines due to the char-at-a-time behaviour: the read consumes them before peek has a chance to. It looks like fixing that may be a bit more involved than what I’ve done here.

diff --git a/src/kaleidoscope/plugin/FocusSerial.cpp b/src/kaleidoscope/plugin/FocusSerial.cpp
index ec0fdce6fb61..c92645fa7fd6 100644
--- a/src/kaleidoscope/plugin/FocusSerial.cpp
+++ b/src/kaleidoscope/plugin/FocusSerial.cpp
@@ -25,6 +25,7 @@ namespace kaleidoscope {
 namespace plugin {
 
 char FocusSerial::command_[32];
+uint8_t i = 0;
 
 void FocusSerial::drain(void) {
   if (Serial.available())
@@ -36,8 +37,10 @@ EventHandlerResult FocusSerial::beforeReportingState() {
   if (Serial.available() == 0)
     return EventHandlerResult::OK;
 
-  uint8_t i = 0;
   do {
+    if (Serial.available() == 0)
+      return EventHandlerResult::OK;
+
     command_[i++] = Serial.read();
 
     if (Serial.peek() == '\n')
@@ -48,6 +51,8 @@ EventHandlerResult FocusSerial::beforeReportingState() {
   else
     command_[i] = '\0';
 
+  i = 0;
+
   Kaleidoscope.onFocusEvent(command_);
 
   Serial.println(F("\r\n."));
diff --git a/src/kaleidoscope/plugin/FocusSerial.h b/src/kaleidoscope/plugin/FocusSerial.h
index ef691c941bd2..f5bc92efa655 100644
--- a/src/kaleidoscope/plugin/FocusSerial.h
+++ b/src/kaleidoscope/plugin/FocusSerial.h
@@ -90,6 +90,7 @@ class FocusSerial : public kaleidoscope::Plugin {
 
  private:
   static char command_[32];
+  static uint8_t i;
 
   static void drain(void);
   static void printBool(bool b);

2 Likes