From ef94658b4bc5b238640a88419243823d9557cf3d Mon Sep 17 00:00:00 2001 From: Gergely Nagy Date: Fri, 23 Nov 2018 12:01:24 +0100 Subject: [PATCH] FocusSerial: New, simplified API Introduce a couple of helper methods that make it easier to work with Focus. These abstract away the dependency on Serial as a side-effect. The intent is that all traffic will go through Focus. Fixes #476. Signed-off-by: Gergely Nagy --- UPGRADING.md | 8 +-- doc/plugin/FocusSerial.md | 32 +++++++++- examples/Features/FocusSerial/FocusSerial.ino | 2 +- src/kaleidoscope/plugin/CycleTimeReport.cpp | 4 +- src/kaleidoscope/plugin/EEPROM-Keymap.cpp | 26 +++----- src/kaleidoscope/plugin/EEPROM-Settings.cpp | 30 +++++---- src/kaleidoscope/plugin/FocusSerial.cpp | 28 +-------- src/kaleidoscope/plugin/FocusSerial.h | 62 ++++++++++++++++--- src/kaleidoscope/plugin/HostOS-Focus.cpp | 8 +-- src/kaleidoscope/plugin/LED-Palette-Theme.cpp | 28 ++++----- src/kaleidoscope/plugin/LEDControl.cpp | 32 +++++----- src/kaleidoscope/plugin/TypingBreaks.cpp | 30 ++++----- 12 files changed, 159 insertions(+), 131 deletions(-) diff --git a/UPGRADING.md b/UPGRADING.md index d65d43a1..8169e739 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -101,7 +101,7 @@ Upgrading from `Focus` to `onFocusEvent` and `FocusSerial` is a reasonably simpl ##### The most trivial example -The biggest difference between `Focus` and `onFocusEvent` is that the former required explicit registering of hooks, while the latter does it automatically: every plugin that implements the `onFocusEvent` method will be part of the system. As a consequence, only plugins are able to supply new commands: there is no explicit registration, thus, no way to inject a command that isn't part of a plugin. This also means that these functions now return `kaleidoscope::EventHandlerResult` instead of `bool`. Lets see a trivial example! +The biggest difference between `Focus` and `onFocusEvent` is that the former required explicit registering of hooks, while the latter does it automatically: every plugin that implements the `onFocusEvent` method will be part of the system. As a consequence, only plugins are able to supply new commands: there is no explicit registration, thus, no way to inject a command that isn't part of a plugin. This also means that these functions now return `kaleidoscope::EventHandlerResult` instead of `bool`. Furthermore, with `FocusSerial`, all communication is expected to go through it, instead of using `Serial` directly. Lets see a trivial example! ###### Focus @@ -136,7 +136,7 @@ class FocusExampleCommand : public Plugin { if (strcmp_P(command, PSTR("example")) != 0) return EventHandlerResult::OK; - Serial.println(F("This is an example response. Hello world!")); + ::Focus.send(F("This is an example response. Hello world!")); return EventHandlerResult::EVENT_CONSUMED; } }; @@ -212,7 +212,7 @@ class ExamplePlugin : public Plugin { return EventHandlerResult::OK; example_toggle_ = !example_toggle_; - ::Focus.printBool(example_toggle_); + ::Focus.send(example_toggle_); return EventHandlerResult::EVENT_CONSUMED; } @@ -271,7 +271,7 @@ class ExampleOptionalCommand : public Plugin { if (strcmp_P(command, PSTR("optional")) != 0) return EventHandlerResult::OK; - Serial.println(Layer.getLayerState(), BIN); + ::Focus.send(Layer.getLayerState()); return EventHandlerResult::EVENT_CONSUMED; } }; diff --git a/doc/plugin/FocusSerial.md b/doc/plugin/FocusSerial.md index 2d32d269..2ff89069 100644 --- a/doc/plugin/FocusSerial.md +++ b/doc/plugin/FocusSerial.md @@ -26,7 +26,7 @@ class FocusTestCommand : public Plugin { if (strcmp_P(command, PSTR("test")) != 0) return EventHandlerResult::OK; - Serial.println(F("Congratulations, the test command works!")); + ::Focus.send(F("Congratulations, the test command works!")); return EventHandlerResult::EVENT_CONSUMED; } }; @@ -43,7 +43,34 @@ void setup () { ## Plugin methods -The plugin provides the `Focus` object, with a couple of helper methods aimed at developers. Documenting those is a work in progress for now. +The plugin provides the `Focus` object, with a couple of helper methods aimed at developers. Terminating the response with a dot on its own line is handled implicitly by `FocusSerial`, one does not need to do that explicitly. + +### `.send(...)` +### `.sendRaw(...)` + +Sends a list of variables over the wire. The difference between `.send()` and `.sendRaw()` is that the former puts a space between each variable, the latter does not. If one just wants to send a list of things, use the former. If one needs more control over the formatting, use the latter. In most cases, `.send()` is the recommended method to use. + +Both of them take a variable number of arguments, of almost any type: all built-in types can be sent, `cRGB`, `Key` and `bool` too in addition. For colors, `.send()` will write them as an `R G B` sequence; `Key` objects will be sent as the raw 16-bit keycode; and `bool` will be sent as either the string `true`, or `false`. + +### `.read(variable)` + +Depending on the type of the variable passed by reference, reads a 8 or 16-bit unsigned integer, a `Key`, or a `cRGB` color from the wire, into the variable passed as the argument. + +### `.peek()` + +Returns the next character on the wire, without reading it. Subsequent reads will include the peeked-at byte too. + +### `.isEOL()` + +Returns whether we're at the end of the request line. + +### `.COMMENT` + +When sending something to the host that is not a response to a request, prefix the response lines with this. + +### `.SEPARATOR` + +To be used when using `.sendRaw`, when one needs complete control over where separators are inserted into the response. ## Wire protocol @@ -61,6 +88,7 @@ Apart from these, there are no restrictions on what can go over the wire, but to * Do as little work in the hooks as possible. While the protocol is human readable, the expectation is that tools will be used to interact with the keyboard. * As such, keep formatting to the bare minimum. No fancy table-like responses. * In general, the output of a getter should be copy-pasteable to a setter. +* Messages sent to the host, without a request, should be prefixed with a hash mark (`Focus.COMMENT`). These are merely guidelines, and there can be - and are - exceptions. Use your discretion when writing Focus hooks. diff --git a/examples/Features/FocusSerial/FocusSerial.ino b/examples/Features/FocusSerial/FocusSerial.ino index 61113b09..26de9f28 100644 --- a/examples/Features/FocusSerial/FocusSerial.ino +++ b/examples/Features/FocusSerial/FocusSerial.ino @@ -51,7 +51,7 @@ class FocusTestCommand : public Plugin { return EventHandlerResult::OK; if (strcmp_P(command, cmd) == 0) { - Serial.println(F("ok!")); + ::Focus.send(F("ok!")); return EventHandlerResult::EVENT_CONSUMED; } diff --git a/src/kaleidoscope/plugin/CycleTimeReport.cpp b/src/kaleidoscope/plugin/CycleTimeReport.cpp index 7401789d..de91df6c 100644 --- a/src/kaleidoscope/plugin/CycleTimeReport.cpp +++ b/src/kaleidoscope/plugin/CycleTimeReport.cpp @@ -16,6 +16,7 @@ */ #include +#include namespace kaleidoscope { namespace plugin { @@ -55,8 +56,7 @@ EventHandlerResult CycleTimeReport::afterEachCycle() { } __attribute__((weak)) void cycleTimeReport(void) { - Serial.print(F("# average loop time: ")); - Serial.println(CycleTimeReport.average_loop_time); + Focus.send(Focus.COMMENT, F("average loop time:"), CycleTimeReport.average_loop_time); } kaleidoscope::plugin::CycleTimeReport CycleTimeReport; diff --git a/src/kaleidoscope/plugin/EEPROM-Keymap.cpp b/src/kaleidoscope/plugin/EEPROM-Keymap.cpp index bb415be2..7a46fdd8 100644 --- a/src/kaleidoscope/plugin/EEPROM-Keymap.cpp +++ b/src/kaleidoscope/plugin/EEPROM-Keymap.cpp @@ -85,18 +85,6 @@ void EEPROMKeymap::updateKey(uint16_t base_pos, Key key) { EEPROM.update(keymap_base_ + base_pos * 2 + 1, key.keyCode); } -Key EEPROMKeymap::parseKey(void) { - Key key; - - key.raw = Serial.parseInt(); - - return key; -} - -void EEPROMKeymap::printKey(Key k) { - ::Focus.printNumber(k.raw); -} - EventHandlerResult EEPROMKeymap::onFocusEvent(const char *command) { const char *cmd = PSTR("keymap.map"); if (::Focus.handleHelp(command, PSTR("keymap.map\nkeymap.roLayers"))) @@ -108,32 +96,32 @@ EventHandlerResult EEPROMKeymap::onFocusEvent(const char *command) { if (strcmp_P(command + 7, PSTR("roLayers")) == 0) { if (mode_ != Mode::EXTEND) return EventHandlerResult::OK; - Serial.println(progmem_layers_); + ::Focus.send(progmem_layers_); return EventHandlerResult::EVENT_CONSUMED; } if (strcmp_P(command + 7, PSTR("map")) != 0) return EventHandlerResult::OK; - if (Serial.peek() == '\n') { + if (::Focus.isEOL()) { for (uint8_t layer = 0; layer < layer_count; layer++) { for (uint8_t row = 0; row < ROWS; row++) { for (uint8_t col = 0; col < COLS; col++) { Key k = Layer.getKey(layer, row, col); - printKey(k); - ::Focus.printSpace(); + ::Focus.send(k); } } } - Serial.println(); } else { uint16_t i = 0; uint8_t layers = layer_count; if (layers > 0) layers--; - while ((Serial.peek() != '\n') && (i < ROWS * COLS * layers)) { - Key k = parseKey(); + while (!::Focus.isEOL() && (i < ROWS * COLS * layers)) { + Key k; + + ::Focus.read(k); if (mode_ == Mode::EXTEND) { uint8_t layer = i / (ROWS * COLS); diff --git a/src/kaleidoscope/plugin/EEPROM-Settings.cpp b/src/kaleidoscope/plugin/EEPROM-Settings.cpp index 9e0e4d72..b75e181e 100644 --- a/src/kaleidoscope/plugin/EEPROM-Settings.cpp +++ b/src/kaleidoscope/plugin/EEPROM-Settings.cpp @@ -143,24 +143,23 @@ EventHandlerResult FocusSettingsCommand::onFocusEvent(const char *command) { switch (sub_command) { case DEFAULT_LAYER: { - if (Serial.peek() == '\n') { - Serial.println(::EEPROMSettings.default_layer()); + if (::Focus.isEOL()) { + ::Focus.send(::EEPROMSettings.default_layer()); } else { - ::EEPROMSettings.default_layer(Serial.parseInt()); + uint8_t layer; + ::Focus.read(layer); + ::EEPROMSettings.default_layer(layer); } break; } case IS_VALID: - ::Focus.printBool(::EEPROMSettings.isValid()); - Serial.println(); + ::Focus.send(::EEPROMSettings.isValid()); break; case GET_VERSION: - Serial.println(::EEPROMSettings.version()); + ::Focus.send(::EEPROMSettings.version()); break; case CRC: - Serial.print(::CRC.crc, HEX); - Serial.print(F("/")); - Serial.println(::EEPROMSettings.crc(), HEX); + ::Focus.sendRaw(::CRC.crc, F("/"), ::EEPROMSettings.crc()); break; } @@ -185,16 +184,15 @@ EventHandlerResult FocusEEPROMCommand::onFocusEvent(const char *command) { switch (sub_command) { case CONTENTS: { - if (Serial.peek() == '\n') { + if (::Focus.isEOL()) { for (uint16_t i = 0; i < EEPROM.length(); i++) { uint8_t d = EEPROM[i]; - ::Focus.printNumber(d); - ::Focus.printSpace(); + ::Focus.send(d); } - Serial.println(); } else { - for (uint16_t i = 0; i < EEPROM.length() && Serial.peek() != '\n'; i++) { - uint8_t d = Serial.parseInt(); + for (uint16_t i = 0; i < EEPROM.length() && !::Focus.isEOL(); i++) { + uint8_t d; + ::Focus.read(d); EEPROM.update(i, d); } } @@ -202,7 +200,7 @@ EventHandlerResult FocusEEPROMCommand::onFocusEvent(const char *command) { break; } case FREE: - Serial.println(EEPROM.length() - ::EEPROMSettings.used()); + ::Focus.send(EEPROM.length() - ::EEPROMSettings.used()); break; } diff --git a/src/kaleidoscope/plugin/FocusSerial.cpp b/src/kaleidoscope/plugin/FocusSerial.cpp index 9c240963..ec0fdce6 100644 --- a/src/kaleidoscope/plugin/FocusSerial.cpp +++ b/src/kaleidoscope/plugin/FocusSerial.cpp @@ -50,7 +50,7 @@ EventHandlerResult FocusSerial::beforeReportingState() { Kaleidoscope.onFocusEvent(command_); - Serial.println(F(".")); + Serial.println(F("\r\n.")); drain(); @@ -74,36 +74,10 @@ EventHandlerResult FocusSerial::onFocusEvent(const char *command) { return EventHandlerResult::OK; } -void FocusSerial::printSpace(void) { - Serial.print(F(" ")); -} - -void FocusSerial::printNumber(uint16_t num) { - Serial.print(num); -} - -void FocusSerial::printColor(uint8_t r, uint8_t g, uint8_t b) { - printNumber(r); - printSpace(); - printNumber(g); - printSpace(); - printNumber(b); -} - -void FocusSerial::printSeparator(void) { - Serial.print(F(" | ")); -} - void FocusSerial::printBool(bool b) { Serial.print((b) ? F("true") : F("false")); } -void FocusSerial::readColor(cRGB &color) { - color.r = Serial.parseInt(); - color.g = Serial.parseInt(); - color.b = Serial.parseInt(); -} - } } diff --git a/src/kaleidoscope/plugin/FocusSerial.h b/src/kaleidoscope/plugin/FocusSerial.h index 3f600c54..ca6f856b 100644 --- a/src/kaleidoscope/plugin/FocusSerial.h +++ b/src/kaleidoscope/plugin/FocusSerial.h @@ -28,14 +28,61 @@ class FocusSerial : public kaleidoscope::Plugin { bool handleHelp(const char *command, const char *help_message); - /* Helpers */ - static void printNumber(uint16_t number); - static void printSpace(void); - static void printColor(uint8_t r, uint8_t g, uint8_t b); - static void printSeparator(void); - static void printBool(bool b); + void send() {} + void send(const cRGB color) { + send(color.r, color.g, color.b); + } + void send(const Key key) { + send(key.raw); + Serial.print(SEPARATOR); + } + void send(const bool b) { + printBool(b); + Serial.print(SEPARATOR); + } + template + void send(V v) { + Serial.print(v); + Serial.print(SEPARATOR); + } + template + void send(Var v, const Vars&... vars) { + send(v); + send(vars...); + } + + void sendRaw() {} + template + void sendRaw(Var v, const Vars&... vars) { + Serial.print(v); + sendRaw(vars...); + } + + const char peek() { + return Serial.peek(); + } - static void readColor(cRGB &color); + void read(Key &key) { + key.raw = Serial.parseInt(); + } + void read(cRGB &color) { + color.r = Serial.parseInt(); + color.g = Serial.parseInt(); + color.b = Serial.parseInt(); + } + void read(uint8_t &u8) { + u8 = Serial.parseInt(); + } + void read(uint16_t &u16) { + u16 = Serial.parseInt(); + } + + bool isEOL() { + return Serial.peek() == '\n'; + } + + static constexpr char COMMENT = '#'; + static constexpr char SEPARATOR = ' '; /* Hooks */ EventHandlerResult onSetup() { @@ -49,6 +96,7 @@ class FocusSerial : public kaleidoscope::Plugin { static char command_[32]; static void drain(void); + static void printBool(bool b); }; } } diff --git a/src/kaleidoscope/plugin/HostOS-Focus.cpp b/src/kaleidoscope/plugin/HostOS-Focus.cpp index 222e1399..fcbfe18d 100644 --- a/src/kaleidoscope/plugin/HostOS-Focus.cpp +++ b/src/kaleidoscope/plugin/HostOS-Focus.cpp @@ -29,14 +29,14 @@ EventHandlerResult FocusHostOSCommand::onFocusEvent(const char *command) { if (strcmp_P(command, cmd) != 0) return EventHandlerResult::OK; - if (Serial.peek() == '\n') { - Serial.println(::HostOS.os()); + if (::Focus.isEOL()) { + ::Focus.send(::HostOS.os()); } else { - uint8_t new_os = Serial.parseInt(); + uint8_t new_os; + ::Focus.read(new_os); ::HostOS.os((hostos::Type) new_os); } - Serial.read(); return EventHandlerResult::EVENT_CONSUMED; } diff --git a/src/kaleidoscope/plugin/LED-Palette-Theme.cpp b/src/kaleidoscope/plugin/LED-Palette-Theme.cpp index ab723567..73eba806 100644 --- a/src/kaleidoscope/plugin/LED-Palette-Theme.cpp +++ b/src/kaleidoscope/plugin/LED-Palette-Theme.cpp @@ -111,25 +111,21 @@ EventHandlerResult LEDPaletteTheme::onFocusEvent(const char *command) { if (strcmp_P(command, cmd) != 0) return EventHandlerResult::OK; - if (Serial.peek() == '\n') { + if (::Focus.isEOL()) { for (uint8_t i = 0; i < 16; i++) { cRGB color; EEPROM.get(palette_base_ + i * sizeof(color), color); - ::Focus.printColor(color.r, color.g, color.b); - ::Focus.printSpace(); + ::Focus.send(color); } - Serial.println(); return EventHandlerResult::EVENT_CONSUMED; } uint8_t i = 0; - while (i < 16 && Serial.peek() != '\n') { + while (i < 16 && !::Focus.isEOL()) { cRGB color; - color.r = Serial.parseInt(); - color.g = Serial.parseInt(); - color.b = Serial.parseInt(); + ::Focus.read(color); EEPROM.put(palette_base_ + i * sizeof(color), color); i++; @@ -155,24 +151,22 @@ EventHandlerResult LEDPaletteTheme::themeFocusEvent(const char *command, uint16_t max_index = (max_themes * ROWS * COLS) / 2; - if (Serial.peek() == '\n') { + if (::Focus.isEOL()) { for (uint16_t pos = 0; pos < max_index; pos++) { uint8_t indexes = EEPROM.read(theme_base + pos); - ::Focus.printNumber(indexes >> 4); - ::Focus.printSpace(); - ::Focus.printNumber(indexes & ~0xf0); - ::Focus.printSpace(); + ::Focus.send((uint8_t)(indexes >> 4), indexes & ~0xf0); } - Serial.println(); return EventHandlerResult::EVENT_CONSUMED; } uint16_t pos = 0; - while ((Serial.peek() != '\n') && (pos < max_index)) { - uint8_t idx1 = Serial.parseInt(); - uint8_t idx2 = Serial.parseInt(); + while (!::Focus.isEOL() && (pos < max_index)) { + uint8_t idx1, idx2; + ::Focus.read(idx1); + ::Focus.read(idx2); + uint8_t indexes = (idx1 << 4) + idx2; EEPROM.update(theme_base + pos, indexes); diff --git a/src/kaleidoscope/plugin/LEDControl.cpp b/src/kaleidoscope/plugin/LEDControl.cpp index b74093af..5f7eb32c 100644 --- a/src/kaleidoscope/plugin/LEDControl.cpp +++ b/src/kaleidoscope/plugin/LEDControl.cpp @@ -214,17 +214,18 @@ EventHandlerResult FocusLEDCommand::onFocusEvent(const char *command) { switch (subCommand) { case AT: { - uint8_t idx = Serial.parseInt(); + uint8_t idx; - if (Serial.peek() == '\n') { + ::Focus.read(idx); + + if (::Focus.isEOL()) { cRGB c = ::LEDControl.getCrgbAt(idx); - ::Focus.printColor(c.r, c.g, c.b); - Serial.println(); + ::Focus.send(c); } else { cRGB c; - ::Focus.readColor(c); + ::Focus.read(c); ::LEDControl.setCrgbAt(idx, c); } @@ -233,46 +234,43 @@ EventHandlerResult FocusLEDCommand::onFocusEvent(const char *command) { case SETALL: { cRGB c; - ::Focus.readColor(c); + ::Focus.read(c); ::LEDControl.set_all_leds_to(c); break; } case MODE: { - char peek = Serial.peek(); + char peek = ::Focus.peek(); if (peek == '\n') { - Serial.println(::LEDControl.get_mode_index()); + ::Focus.send(::LEDControl.get_mode_index()); } else if (peek == 'n') { ::LEDControl.next_mode(); - Serial.read(); } else if (peek == 'p') { ::LEDControl.prev_mode(); - Serial.read(); } else { - uint8_t mode = Serial.parseInt(); + uint8_t mode; + ::Focus.read(mode); ::LEDControl.set_mode(mode); } break; } case THEME: { - if (Serial.peek() == '\n') { + if (::Focus.isEOL()) { for (int8_t idx = 0; idx < LED_COUNT; idx++) { cRGB c = ::LEDControl.getCrgbAt(idx); - ::Focus.printColor(c.r, c.g, c.b); - ::Focus.printSpace(); + ::Focus.send(c); } - Serial.println(); break; } int8_t idx = 0; - while (idx < LED_COUNT && Serial.peek() != '\n') { + while (idx < LED_COUNT && !::Focus.isEOL()) { cRGB color; - ::Focus.readColor(color); + ::Focus.read(color); ::LEDControl.setCrgbAt(idx, color); idx++; diff --git a/src/kaleidoscope/plugin/TypingBreaks.cpp b/src/kaleidoscope/plugin/TypingBreaks.cpp index 071fc678..a995ff44 100644 --- a/src/kaleidoscope/plugin/TypingBreaks.cpp +++ b/src/kaleidoscope/plugin/TypingBreaks.cpp @@ -165,38 +165,38 @@ EventHandlerResult TypingBreaks::onFocusEvent(const char *command) { switch (subCommand) { case IDLE_TIME_LIMIT: - if (Serial.peek() == '\n') { - Serial.println(settings.idle_time_limit); + if (::Focus.isEOL()) { + ::Focus.send(settings.idle_time_limit); } else { - settings.idle_time_limit = Serial.parseInt(); + ::Focus.read(settings.idle_time_limit); } break; case LOCK_TIMEOUT: - if (Serial.peek() == '\n') { - Serial.println(settings.lock_time_out); + if (::Focus.isEOL()) { + ::Focus.send(settings.lock_time_out); } else { - settings.lock_time_out = Serial.parseInt(); + ::Focus.read(settings.lock_time_out); } break; case LOCK_LENGTH: - if (Serial.peek() == '\n') { - Serial.println(settings.lock_length); + if (::Focus.isEOL()) { + ::Focus.send(settings.lock_length); } else { - settings.lock_length = Serial.parseInt(); + ::Focus.read(settings.lock_length); } break; case LEFT_MAX: - if (Serial.peek() == '\n') { - Serial.println(settings.left_hand_max_keys); + if (::Focus.isEOL()) { + ::Focus.send(settings.left_hand_max_keys); } else { - settings.left_hand_max_keys = Serial.parseInt(); + ::Focus.read(settings.left_hand_max_keys); } break; case RIGHT_MAX: - if (Serial.peek() == '\n') { - Serial.println(settings.right_hand_max_keys); + if (::Focus.isEOL()) { + ::Focus.send(settings.right_hand_max_keys); } else { - settings.right_hand_max_keys = Serial.parseInt(); + ::Focus.read(settings.right_hand_max_keys); } break; }