Fix tests

Author: loopj

firmware/libsi/include/si/device/commands.h

@@ -16,16 +16,35 @@
  */
 typedef int (*si_command_handler_fn)(const uint8_t *command, si_complete_cb_t callback, void *context);
 
+/**
+ * Command structure representing a registered command.
+ */
+struct si_command {
+  uint8_t command;
+  uint8_t length;
+  si_command_handler_fn handler;
+  void *user_data;
+};
+
 /**
  * Register a command handler for commands from an SI host.
  *
  * @param command the command to handle
- * @param command_length the length of the command
+ * @param command_length the length of the command in bytes
  * @param handler the command handler function
  *
  */
 void si_command_register(uint8_t command, uint8_t length, si_command_handler_fn handler, void *context);
 
+/**
+ * Look up a command structure by command ID.
+ *
+ * @param command the command ID to look up
+ *
+ * @return pointer to the command structure, or NULL if not found
+ */
+struct si_command *si_command_find_by_id(uint8_t command);
+
 /**
  * Process a single SI command on the bus.
  *

firmware/libsi/src/device/commands.c

@@ -9,16 +9,9 @@ enum {
   BUS_STATE_ERROR,
 };
 
-struct command_entry {
-  uint8_t command;
-  uint8_t length;
-  si_command_handler_fn handler;
-  void *context;
-};
-
 // Command table for registered SI commands
-static struct command_entry command_table[COMMAND_TABLE_SIZE] = {0};
-static struct command_entry *current_command                  = NULL;
+static struct si_command command_table[COMMAND_TABLE_SIZE] = {0};
+static struct si_command *current_command                  = NULL;
 
 // Current bus state
 static uint8_t bus_state = BUS_STATE_UNKNOWN;
@@ -43,20 +36,6 @@ static inline uint8_t hash_command(uint8_t command)
   return (command % (COMMAND_TABLE_SIZE - 2)) + 2;
 }
 
-// Find a command entry by command id
-static struct command_entry *find_command(uint8_t command)
-{
-  uint8_t index = hash_command(command);
-
-  while (command_table[index].handler != NULL) {
-    if (command_table[index].command == command) {
-      return &command_table[index];
-    }
-    index = (index + 1) % COMMAND_TABLE_SIZE;
-  }
-  return NULL;
-}
-
 // Command handler TX completion callback
 static void on_tx_complete(int result)
 {
@@ -73,7 +52,7 @@ static void on_rx_complete(int result)
 {
   // If we successfully read a command and have a handler, call it
   if (result >= 0 && current_command && current_command->handler) {
-    current_command->handler(command_buffer, on_tx_complete, current_command->context);
+    current_command->handler(command_buffer, on_tx_complete, current_command->user_data);
     return;
   }
 
@@ -87,22 +66,21 @@ static void on_rx_complete(int result)
 
 static bool command_byte_cb(uint8_t byte, uint8_t byte_index)
 {
-  // If this is the first byte, look up the command entry
+  // If this is the first byte, check if there is a registered command
   if (byte_index == 0) {
-    current_command = find_command(byte);
-    if (current_command == NULL) {
-      return false; // Stop reading on unknown command
-    }
+    current_command = si_command_find_by_id(byte);
+    if (current_command == NULL)
+      return false;
   }
 
-  if (byte_index == current_command->length - 1) {
-    return false; // Stop reading after the expected length
-  }
+  // Stop reading when we reach the expected length
+  if (byte_index == current_command->length - 1)
+    return false;
 
   return true;
 }
 
-void si_command_register(uint8_t command, uint8_t length, si_command_handler_fn handler, void *context)
+void si_command_register(uint8_t command, uint8_t length, si_command_handler_fn handler, void *user_data)
 {
   uint8_t index = hash_command(command);
 
@@ -113,15 +91,28 @@ void si_command_register(uint8_t command, uint8_t length, si_command_handler_fn
     index = (index + 1) % COMMAND_TABLE_SIZE;
   }
 
-  // Store the command entry
-  command_table[index] = (struct command_entry){
-      .command = command,
-      .length  = length,
-      .handler = handler,
-      .context = context,
+  // Store the command
+  command_table[index] = (struct si_command){
+      .command   = command,
+      .length    = length,
+      .handler   = handler,
+      .user_data = user_data,
   };
 }
 
+struct si_command *si_command_find_by_id(uint8_t command)
+{
+  uint8_t index = hash_command(command);
+
+  while (command_table[index].handler != NULL) {
+    if (command_table[index].command == command) {
+      return &command_table[index];
+    }
+    index = (index + 1) % COMMAND_TABLE_SIZE;
+  }
+  return NULL;
+}
+
 void si_command_process()
 {
   if (bus_state != BUS_STATE_IDLE)

firmware/libsi/test/test_commands.c

@@ -20,19 +20,25 @@ static int handle_reset(const uint8_t *command, si_complete_cb_t callback, void
 
 static void test_register_command()
 {
+  struct si_command *cmd = NULL;
+
   si_command_register(0x00, 1, handle_info, NULL);
-  TEST_ASSERT_EQUAL(1, si_command_get_length(0x00));
-  TEST_ASSERT_EQUAL(handle_info, si_command_get_handler(0x00));
+  cmd = si_command_find_by_id(0x00);
+  TEST_ASSERT_NOT_NULL(cmd);
+  TEST_ASSERT_EQUAL(1, cmd->length);
+  TEST_ASSERT_EQUAL(handle_info, cmd->handler);
 
   si_command_register(0xFF, 3, handle_reset, NULL);
-  TEST_ASSERT_EQUAL(3, si_command_get_length(0xFF));
-  TEST_ASSERT_EQUAL(handle_reset, si_command_get_handler(0xFF));
+  cmd = si_command_find_by_id(0xFF);
+  TEST_ASSERT_NOT_NULL(cmd);
+  TEST_ASSERT_EQUAL(3, cmd->length);
+  TEST_ASSERT_EQUAL(handle_reset, cmd->handler);
 }
 
 static void test_register_command_missing()
 {
-  TEST_ASSERT_EQUAL(0, si_command_get_length(0x69));
-  TEST_ASSERT_NULL(si_command_get_handler(0x69));
+  struct si_command *cmd = si_command_find_by_id(0x69);
+  TEST_ASSERT_NULL(cmd);
 }
 
 void test_commands(void)

firmware/libsi/test/test_gc_controller.c

@@ -17,15 +17,15 @@ void si_write_bytes(const uint8_t *data, uint8_t length, si_complete_cb_t callba
   response_len = length;
 }
 
-void si_read_command(uint8_t *data, si_complete_cb_t callback)
+void si_read_bytes(uint8_t *buffer, uint8_t max_length, si_byte_cb_t byte_callback, si_complete_cb_t complete_callback)
 {
 }
 
 // Simulate receiving a command
 static int simulate_command(struct si_device_gc_controller *device, uint8_t *command)
 {
-  si_command_handler_fn handler = si_command_get_handler(command[0]);
-  return handler(command, NULL, device);
+  struct si_command *cmd = si_command_find_by_id(command[0]);
+  return cmd->handler(command, NULL, device);
 }
 
 // Test that the device info response is correct for a standard GameCube controller