fix(control)!: unify response frame format

README.md

@@ -59,20 +59,26 @@ When connected the emberOne usbserial firmware will create two serial ports. Usu
 - First serial port
 - baudrate does not matter
 
-**Packet Format**
+**Framing**
+
+Packets are length-prefixed with no start-of-frame marker or
+checksum. If framing gets out of sync, a silent gap on the
+bus (currently 4 ms) resets the firmware's receive state and
+resynchronizes the link.
+
+**Command Packet Format**
 
 | 0      | 1      | 2  | 3   | 4    | 5   | 6... |
 |--------|--------|----|-----|------|-----|------|
 | LEN LO | LEN HI | ID | BUS | PAGE | CMD | DATA |
 
 ```
-0. length low
-1. length high
-	- packet length is number of bytes of the whole packet. 
+0-1. length (u16 LE)
+	- total packet length
 2. command id
-	- Whatever byte you want. will be returned in the response 
+	- echoed in the response
 3. command bus
-	- always 0x00 
+	- always 0x00
 4. command page
 	- I2C:  0x05
 	- GPIO: 0x06
@@ -84,20 +90,57 @@ When connected the emberOne usbserial firmware will create two serial ports. Usu
 	- data to write. variable length. See below
 ```
 
+**Response Packet Format**
+
+Every command produces a response with the
+command ID echoed back so the host can match responses to
+requests.
+
+| 0      | 1      | 2  | 3    | 4... |
+|--------|--------|----|------|------|
+| LEN LO | LEN HI | ID | CODE | DATA |
+
+```
+0-1. length (u16 LE)
+	- total packet length, same convention as commands
+2. command id
+	- echoed from the request, or 0xFF if the command
+	  could not be parsed
+3. status code
+	- 0x00: Success (DATA contains the response payload)
+	- 0x10: Timeout
+	- 0x11: Invalid command
+	- 0x12: Buffer overflow
+	- 0xFF: Error with message (DATA contains ASCII text)
+4+. data
+	- response payload or error message, varies by command
+```
+
 **I2C**
 
 Commands:
 
+- set frequency: 0x10
 - write: 0x20
 - read: 0x30
 - readwrite: 0x40
 
 Data:
 
-- [I2C address, (bytes to write), (number of bytes to read)]
+- set frequency: [frequency (u32 LE)]
+- write/read/readwrite: [I2C address, (bytes to write),
+  (number of bytes to read)]
+
+Response data:
+
+- set frequency: echoed frequency (4 bytes, u32 LE)
+- write: number of bytes written (1 byte)
+- read: bytes read from device
+- readwrite: bytes read from device
 
 Example:
 
+- set I2C frequency to 400 kHz: `0A 00 01 00 05 10 80 1A 06 00`
 - write 0xDE to addr 0x4F: `08 00 01 00 05 20 4F DE`
 - read one byte from addr 0x4C: `08 00 01 00 05 30 4C 01`
 - readwrite two bytes from addr 0x32, reg 0xFE: `09 00 01 00 05 40 32 FE 02`
@@ -115,6 +158,10 @@ Data:
 - [pin level] (0 = low, 1 = high)
 - omit data to read current level
 
+Response data:
+
+- current pin level after the operation (1 byte, 0 or 1)
+
 Examples:
 
 - Set ASIC Reset High: `07 00 00 00 06 00 01`
@@ -134,6 +181,10 @@ Commands:
 - read VDD: 0x50
 - read VIN: 0x51
 
+Response data:
+
+- raw ADC value (2 bytes, u16 LE)
+
 Example:
 
 - read VDD Pin: `06 00 00 00 07 50`
@@ -148,6 +199,10 @@ Data:
 
 - [R, G, B]
 
+Response data:
+
+- echoed R, G, B (3 bytes)
+
 Example:
 
 - Set LED Magenta: `09 00 00 00 08 10 FF 00 FF`

src/control/led.rs

@@ -1,5 +1,5 @@
 use embassy_rp::{
-    peripherals::{PIO0},
+    peripherals::PIO0,
     pio::{Common, PioPin, StateMachine},
     pio_programs::ws2812::{PioWs2812, PioWs2812Program},
 };

src/control/mod.rs

@@ -22,6 +22,14 @@ const ADC_COMMAND: u8 = 7;
 pub mod led;
 const LED_COMMAND: u8 = 8;
 
+#[repr(u8)]
+enum Status {
+    Success = 0x00,
+    Timeout = 0x10,
+    Invalid = 0x11,
+    BufferOverflow = 0x12,
+    Message = 0xFF,
+}
 
 #[derive(defmt::Format)]
 struct Command {
@@ -70,36 +78,27 @@ impl Command {
 
 #[derive(defmt::Format)]
 pub enum CommandError {
-    Timeout,               // 0x10
-    Invalid,               // 0x11
-    BufferOverflow,        // 0x12
-    Message(&'static str), // 0xff
+    Timeout,
+    Invalid,
+    BufferOverflow,
+    Message(&'static str),
 }
 
 impl CommandError {
-    fn to_bytes(&self) -> Vec<u8, 260> {
-        let mut buf = Vec::<u8, 260>::new();
-        buf.extend_from_slice(&[0x00, 0x00, 0xff]).unwrap();
-
+    fn status(&self) -> Status {
         match self {
-            CommandError::Timeout => {
-                buf.push(0x10).unwrap();
-            }
-            CommandError::Invalid => {
-                buf.push(0x11).unwrap();
-            }
-            CommandError::BufferOverflow => {
-                buf.push(0x12).unwrap();
-            }
-            CommandError::Message(msg) => {
-                buf.push(0xff).unwrap();
-                buf.extend_from_slice(msg.as_bytes()).unwrap();
-            }
+            CommandError::Timeout => Status::Timeout,
+            CommandError::Invalid => Status::Invalid,
+            CommandError::BufferOverflow => Status::BufferOverflow,
+            CommandError::Message(_) => Status::Message,
         }
+    }
 
-        let len = (buf.len() as u16).to_le_bytes();
-        buf[0..2].clone_from_slice(&len);
-        buf
+    fn message(&self) -> &[u8] {
+        match self {
+            CommandError::Message(msg) => msg.as_bytes(),
+            _ => &[],
+        }
     }
 }
 
@@ -117,6 +116,17 @@ pub trait ControllerCommand {
     async fn handle(&self, controller: &mut Controller) -> Result<Vec<u8, 256>, CommandError>;
 }
 
+fn build_response(id: u8, status: Status, data: &[u8]) -> Vec<u8, 260> {
+    let mut buf = Vec::<u8, 260>::new();
+    buf.extend_from_slice(&[0x00, 0x00]).unwrap();
+    buf.push(id).unwrap();
+    buf.push(status as u8).unwrap();
+    buf.extend_from_slice(data).unwrap();
+    let len = (buf.len() as u16).to_le_bytes();
+    buf[0..2].clone_from_slice(&len);
+    buf
+}
+
 impl Controller {
     pub async fn run(&mut self) {
         loop {
@@ -129,19 +139,10 @@ impl Controller {
                 CommandInner::Error(err) => Err(err),
             };
 
+            let id = cmd.id as u8;
             let buf = match res {
-                Ok(res) => {
-                    let mut buf = Vec::<u8, 260>::new();
-                    buf.extend_from_slice(&(res.len() as u16).to_le_bytes()).unwrap();
-                    buf.push(cmd.id as u8).unwrap();
-                    buf.extend_from_slice(&res).unwrap();
-                    buf
-                }
-                Err(err) => {
-                    let mut buf = err.to_bytes();
-                    buf[2] = cmd.id as u8;
-                    buf
-                }
+                Ok(res) => build_response(id, Status::Success, &res),
+                Err(err) => build_response(id, err.status(), err.message()),
             };
 
             let _ = self.tx.write_packet(&buf).await;

src/main.rs

@@ -46,7 +46,7 @@ static PRODUCT: &str = "EmberOne00";
 /// USB bcdDevice in BCD format: 0xJJMN = version JJ.M.N
 /// Major = hardware revision, minor.patch = firmware version.
 /// Firmware version restarts at 0.0 with each hardware revision.
-const VERSION: u16 = 0x0500;
+const VERSION: u16 = 0x05_1_0;
 
 
 /// Return a unique serial number for this device by hashing its flash unique ID.