Review.

.github/dependabot.yml

@@ -0,0 +1,8 @@
+version: 2
+updates:
+  - package-ecosystem: "github-actions" # Necessary to update action hashs.
+    directory: "/"
+    schedule:
+      interval: "weekly"
+    # Allow up to 3 opened pull requests for github-actions versions.
+    open-pull-requests-limit: 3

src/devices/base.toit

@@ -4,15 +4,15 @@ import uart
 import io
 
 /*
-An interface representing a Lightbug device
+An interface representing a Lightbug device.
 */
 interface Device extends Comms:
 
 /*
-An interface for communicationg to and from a Lightbug device
+An interface for communicating to and from a Lightbug device.
 */
 interface Comms:
-  // Reader reading from the device
+  // Reader reading from the device.
   in -> io.Reader
-  // Writer writing to the device
-  out -> io.Writer
+  // Writer writing to the device.
+  out -> io.Writer

src/devices/devices.toit

@@ -2,6 +2,4 @@ import .base show Device Comms
 import .fake show Fake
 import .rh2 show RH2
 
-export Device Comms
-export Fake
-export RH2
+export *

src/devices/fake.toit

@@ -3,21 +3,23 @@ import io
 import .base
 import .i2c
 
-// A fake device, that might be useful sometimes while testing
+// A fake device, that might be useful sometimes while testing.
 class Fake implements Device:
   in -> io.Reader:
     return FakeReader
   out -> io.Writer:
     return FakeWriter
 
-class FakeReader extends io.Reader with io.InMixin:
+// The FakeReader doesn't need an additional `.in` method.
+class FakeReader extends io.Reader:
   constructor:
 
   read_ -> ByteArray?:
       // log.debug "FakeReader: Simulating read operation"
       return #[]
 
-class FakeWriter extends io.Writer with io.OutMixin:
+// The FakeWriter doesn't need an additional `.out` method.
+class FakeWriter extends io.Writer:
   try-write_ data/io.Data from/int to/int -> int:
     bytes/ByteArray := ?
     if data is ByteArray:

src/devices/i2c.toit

@@ -4,85 +4,90 @@ import io
 import log
 import io.byte-order show LITTLE-ENDIAN
 
-I2C_ADDRESS_LIGHTBUG := 0x1b
-
-I2C_COMMAND_LIGHTBUG_READABLE_BYTES := 0x01 // Get the number of bytes available to read
-I2C_COMMAND_LIGHTBUG_READ := 0x02 // Read data
-I2C_COMMAND_LIGHTBUG_WRITE := 0x03 // Write data
-
-LBI2CDevice --sda/int --scl/int -> i2c.Device:
+I2C-ADDRESS-LIGHTBUG := 0x1b
+
+I2C-COMMAND-LIGHTBUG-READABLE-BYTES := 0x01 // Get the number of bytes available to read.
+I2C-COMMAND-LIGHTBUG-READ := 0x02 // Read data.
+I2C-COMMAND-LIGHTBUG-WRITE := 0x03 // Write data.
+
+// this is not really good style.
+// The function was upper-case which made it look like a type.
+// Furthermore, the pins to the i2c bus are allocated but never closed.
+// Typically this isn't an issue, as most users keep the i2c device forever, but
+// in theory you should close the pins when you are done with the i2c device.
+lb-i2c-device --sda/int --scl/int -> i2c.Device:
   bus := i2c.Bus
     --sda=gpio.Pin sda
     --scl=gpio.Pin scl
     --frequency=400_000
-  return bus.device I2C_ADDRESS_LIGHTBUG
+  return bus.device I2C-ADDRESS-LIGHTBUG
 
-class Reader extends io.Reader with io.InMixin:
+// No need to add the in-mixin. A reader doesn't need any `.in` method.
+class Reader extends io.Reader:
   device /i2c.Device
   finishWhenEmpty_ /bool
 
-  constructor d/i2c.Device --finishWhenEmpty=false:
-    device = d
+  constructor .device --finishWhenEmpty=false:
     // XXX: --finishWhenEmpty is not used since factoring out into the lightbug package
     // TODO: Decide if we want to keep it, refactor it, or remove it...
     finishWhenEmpty_ = finishWhenEmpty
 
   /**
   Reads the next bytes.
   There are no yields in this function, so it will block until there are bytes to read,
-  as we want to empty the buffer as soon as possible into our own memory.
+    as we want to empty the buffer as soon as possible into our own memory.
   */
   read_ -> ByteArray?:
     // log.debug "calling read_ in LB Reader for i2c"
     all := #[]
-    allExpected := 0
+    all-expected := 0
     loops := 0
-    // Read from the buffer as fast as possible (as our buffer is bigger)
-    // At most 5*(tx buffer), so 5*1000 = 5KB
+    // Read from the buffer as fast as possible (as our buffer is bigger).
+    // At most 5*(tx buffer), so 5*1000 = 5KB.
     while loops <= 5:
       loops++
       // log.debug "Getting number of bytes available to read, loop $loops"
-      lenBytes := device.read-address #[I2C_COMMAND_LIGHTBUG_READABLE_BYTES] 2
-      lenInt := LITTLE-ENDIAN.uint16 lenBytes 0
-      allExpected = allExpected + lenInt
-
+      len-bytes := device.read-address #[I2C-COMMAND-LIGHTBUG-READABLE-BYTES] 2
+      len-int := LITTLE-ENDIAN.uint16 len-bytes 0
+      all-expected = all-expected + len-int
+
+      // --- This comment looks stale.
       // Taking uart as an example, if there are no bytes, it loops until there are some.
       // uart does this with a read state, for now we will just sleep a bit...
-      if lenInt == 0:
+      if len-int == 0:
         if finishWhenEmpty_:
           log.debug "No bytes to read, finishing"
           return null
         // log.debug "No bytes to read, yielding"
         break // Leave the while loop
 
-      log.debug "Got $lenInt bytes to read"
+      log.debug "Got $len-int bytes to read"
 
-      while lenInt > 0:
-        chunkSize := min lenInt 254
-        log.debug "Reading chunk of $chunkSize bytes stage 1"
-        device.write #[I2C_COMMAND_LIGHTBUG_READ, chunkSize]
-        log.debug "Reading chunk of $chunkSize bytes stage 2"
-        b := device.read chunkSize
-        if b.size != chunkSize:
-          log.error "Failed to read chunk $chunkSize bytes, got $b.size bytes"
+      while len-int > 0:
+        chunk-size := min len-int 254
+        log.debug "Reading chunk of $chunk-size bytes stage 1"
+        device.write #[I2C-COMMAND-LIGHTBUG-READ, chunk-size]
+        log.debug "Reading chunk of $chunk-size bytes stage 2"
+        b := device.read chunk-size
+        if b.size != chunk-size:
+          log.error "Failed to read chunk $chunk-size bytes, got $b.size bytes"
           return null
         all += b
-        lenInt -= chunkSize
+        len-int -= chunk-size
 
-      if all.size != allExpected:
-        log.error "Failed to read $allExpected bytes, got $all.size bytes"
+      if all.size != all-expected:
+        log.error "Failed to read $all-expected bytes, got $all.size bytes"
         return null
 
       log.debug "Read $all.size bytes after $loops loops"
 
-    yield // They are in our buffer now, so yield briefly before returning
+    yield // They are in our buffer now, so yield briefly before returning.
     return all
 
-class Writer extends io.Writer with io.OutMixin:
+class Writer extends io.Writer:
   device /i2c.Device
 
-  constructor d/i2c.Device:
-    device = d
+  constructor .device:
 
   /**
   Writes the given $data to this writer.
@@ -97,21 +102,14 @@ class Writer extends io.Writer with io.OutMixin:
       bytes = ByteArray.from data
     log.debug "Writing $bytes.size bytes"
     log.debug "Bytes: $bytes"
-
-    currentIndex := 0
-    readToIndex := 0
-    while currentIndex < bytes.size:
-      // Send in batches of 255
-      readToIndex = currentIndex + 255
-      if readToIndex > bytes.size:
-        readToIndex = bytes.size
-
-      log.debug "Writing bytes $currentIndex to $readToIndex"
-      log.debug "Bytes: $bytes[currentIndex..readToIndex]"
+
+    List.chunk-up 0 bytes.size 255: | from/int to/int |
+      log.debug "Writing bytes $from to $to"
+      log.debug "Bytes: $bytes[from..to]"
       sendLen := #[0]
-      LITTLE-ENDIAN.put-uint8 sendLen 0 (readToIndex - currentIndex)
-      device.write-address #[I2C_COMMAND_LIGHTBUG_WRITE] sendLen + bytes[currentIndex..readToIndex]
+      LITTLE-ENDIAN.put-uint8 sendLen 0 (to - from)
+      device.write-address #[I2C-COMMAND-LIGHTBUG-WRITE] sendLen + bytes[from..to]
+
+      from = to
 
-      currentIndex = readToIndex
-
-    return bytes.size
+    return bytes.size

src/devices/rh2.toit

@@ -3,14 +3,17 @@ import io
 import .base
 import .i2c
 
-// The RH2 device, currently at revision 3
+// Needs more information. Can't say if abbreviating is OK or not.
+// Most likely, it should be `Rh2`.
+// The RH2 device, currently at revision 3.
 class RH2 implements Device:
-  static I2C_SDA := 6
-  static I2C_SCL := 7
-  static I2C_DEVICE := LBI2CDevice --sda=RH2.I2C_SDA --scl=RH2.I2C_SCL
-  static I2C_READER := Reader I2C_DEVICE
-  static I2C_WRITER := Writer I2C_DEVICE
+  static I2C-SDA := 6
+  static I2C-SCL := 7
+  // Feels weird to have a static and instance way of accessing the in/out.
+  static I2C-DEVICE := lb-i2c-device --sda=RH2.I2C-SDA --scl=RH2.I2C-SCL
+  static I2C-READER := Reader I2C-DEVICE
+  static I2C-WRITER := Writer I2C-DEVICE
   in -> io.Reader:
-    return I2C_READER
+    return I2C-READER
   out -> io.Writer:
-    return I2C_WRITER
+    return I2C-WRITER

src/devices/uart.toit

@@ -3,11 +3,14 @@ import gpio
 
 // ESP32-C6 https://docs.espressif.com/projects/esp-at/en/latest/esp32c6/Get_Started/Hardware_connection.html#esp32c6-4mb-series
 // UART0 GPIO17 (RX) GPIO16 (TX) Defaults
-ESP32C6_UART_RX_PIN := 17
-ESP32C6_UART_TX_PIN := 16
+// Toit typically doesn't "force" users to the defaults. The esp gpio
+// matrix is pretty good. Usually you don't care for the default pins.
+ESP32C6-UART-RX-PIN := 17
+ESP32C6-UART-TX-PIN := 16
 
-ESP32C6UartPort -> uart.Port:
+// As for the i2c device, the pins are never closed.
+esp32c6-uart-port -> uart.Port:
   return uart.Port
-    --rx=gpio.Pin ESP32C6_UART_RX_PIN
-    --tx=gpio.Pin ESP32C6_UART_TX_PIN
-    --baud_rate=115200
+    --rx=gpio.Pin ESP32C6-UART-RX-PIN
+    --tx=gpio.Pin ESP32C6-UART-TX-PIN
+    --baud-rate=115200

src/messages/ack.toit

@@ -0,0 +1,13 @@
+import ..protocol as protocol
+
+class Ack extends protocol.Data:
+  // Do users need this constant?
+  // Should it be private instead? `MT_`
+  // Same for the other messages.
+  static MT := 5
+
+  constructor:
+    super
+
+  msg -> protocol.Message:
+    return protocol.Message.with-data MT this

src/messages/Config.toit → src/messages/config.toit

@@ -5,14 +5,14 @@ class Config extends protocol.Data:
   static KEY := 7
   static PAYLOAD := 9
 
-  constructor.fromData data/protocol.Data:
-    super.fromData data
-  
+  constructor.from-data data/protocol.Data:
+    super.from-data data
+
   msg -> protocol.Message:
-    return protocol.Message.withData MT this
+    return protocol.Message.with-data MT this
 
   key -> int:
-    return getDataUintn KEY
+    return get-data-uintn KEY
 
   payload -> ByteArray:
-    return getData PAYLOAD
+    return get-data PAYLOAD

src/messages/gps-control.toit

@@ -0,0 +1,18 @@
+import ..protocol as protocol
+
+class GPSControl extends protocol.Data:
+  static MT := 39
+  static GPS-ENABLE := 1
+  static RTK-ENABLE-CORRECTION := 2
+
+  constructor --gps/bool --rtk/bool:
+    this.add-data-uint8 GPS-ENABLE (gps ? 1 : 0)
+    this.add-data-uint8 RTK-ENABLE-CORRECTION (rtk ? 1 : 0)
+
+  constructor.from-data data/protocol.Data:
+    super.from-data data
+
+  msg -> protocol.Message:
+    msg := protocol.Message.with-data MT this
+    msg.header.data.add-data-uint8 protocol.Header.TYPE-MESSAGE-METHOD protocol.Header.METHOD-SET
+    return msg