merge main into wip/mc

14 files changed, 188 insertions, 201 deletions

diff --git a/main/CMakeLists.txt b/main/CMakeLists.txt
index d6cbf33..76b1244 100644
--- a/main/CMakeLists.txt
+++ b/main/CMakeLists.txt
@@ -10,7 +10,6 @@ include(lib/FreeRTOS-Kernel/portable/ThirdParty/GCC/RP2040/FreeRTOS_Kernel_impor
 add_subdirectory(lib/mpack)
 add_subdirectory(lib/i2ctcp)
 add_subdirectory(lib/pbdrv)
-include(lib/pbdrv/ext/stdlib/include.cmake) # pico-stdlib is compatible with C stdlib
 
 project(puzzlebox_main C CXX ASM)
 
@@ -24,7 +23,6 @@ add_executable(main
 	mod.c
 	tasks.c
 	blink.c
-	pbdrv.c
 	)
 
 pico_enable_stdio_usb(main 1)
@@ -34,7 +32,7 @@ pico_add_extra_outputs(main)
 # include_directories(lib/pico-sdk/lib/lwip/contrib/ports/freertos/include)
 
 target_include_directories(main PRIVATE .)
-target_link_libraries(main
+target_link_libraries(main PUBLIC
 	pico_cyw43_arch_lwip_sys_freertos
 	pico_stdlib
 	pico_i2c_slave
diff --git a/main/blink.h b/main/blink.h
index 51c5f32..f8262d0 100644
--- a/main/blink.h
+++ b/main/blink.h
@@ -1,4 +1,16 @@
 #pragma once
 
+/**
+ * \ingroup main_tasks
+ * \{
+ */
+
+/**
+ * \brief Status LED blink task
+ *
+ * This task is started after initialization is complete, and blinks the
+ * on-board LED to indicate the Pico has completed initialization successfully.
+ */
 void blink_task();
 
+/// \}
diff --git a/main/config.def.h b/main/config.def.h
index e9503ed..cb6e8b4 100644
--- a/main/config.def.h
+++ b/main/config.def.h
@@ -3,31 +3,69 @@
 #include <cyw43_country.h>
 
 /**
+ * \ingroup main
+ * \defgroup main_config Config
+ * \brief Configuration options
+ *
+ * The main controller firmware is configured statically (i.e. through
+ * compile-time defined options). Because the configuration is likely to
+ * contain Wi-Fi credentials, this file is not tracked under version control.
+ *
+ * Before compiling the main controller fimrware, a file (`config.h`) must be
+ * created by the user with the following format:
+ *
+ * ```c
+ * #pragma once
+ *
+ * // define non-default options here
+ *
+ * #include "config.def.h"
+ * ```
+ *
+ * \note `config.def.h` contains preprocessor logic that tries to ensure a
+ * correct configuration. The default configuration has the following settings:
+ * - Wi-Fi is disabled (prints a warning during compilation because it was not
+ *   explicitly disabled by the user)
+ * - The TCP server is disabled (due to Wi-Fi being disabled)
+ *
+ * \note The exact default values of each configuration option, and all
+ * available options are listed below.
+ *
+ * \{
+ */
+
+/**
  * \name Network (Wi-Fi) configuration
  * \{
  */
 #ifndef CFG_NET_SSID
-//! network name (SSID)
+/**
+ * \brief Network name (SSID)
+ * \note Not defining \c CFG_NET_SSID will implicitly enable \c CFG_NET_DISABLE
+ */
 #define CFG_NET_SSID ""
 #ifndef CFG_NET_DISABLE
-//! disable network communication
+/**
+ * \brief Disable network communication completely
+ * \note Enabling this option will implicitly enable \c CFG_SRV_DISABLE
+ */
 #define CFG_NET_DISABLE
 #warning No SSID defined! Disabling network communication!
 #endif
 #endif
 
 #ifndef CFG_NET_PASS
-//! network password
+//! Network password
 #define CFG_NET_PASS ""
 #endif
 
 #ifndef CFG_NET_AUTH
-//! network security type
+//! Network security type
 #define CFG_NET_AUTH CYW43_AUTH_OPEN
 #endif
 
 #ifndef CFG_NET_CONN_TIMEOUT
-//! max duration (milliseconds) for establishing wifi connection
+//! Max duration (milliseconds) for establishing Wi-Fi connection
 #define CFG_NET_CONN_TIMEOUT 10e3
 #endif
 
@@ -36,47 +74,60 @@
 #define CFG_NET_COUNTRY CYW43_COUNTRY_WORLDWIDE
 #endif
 #ifndef CFG_NET_COUNTRY
-//! radio communications country
+//! Radio communications country
 #define CFG_NET_COUNTRY CYW43_COUNTRY_NETHERLANDS
 #endif
-/** \} */
+/// \}
 
 /**
- * \name i2ctcp server configuration
+ * \name TCP server configuration
  * \{
  */
 #ifndef CFG_SRV_PORT
-//! i2ctcp server port
+//! TCP server port
 #define CFG_SRV_PORT 9191
 #endif
-
 #ifdef CFG_NET_DISABLE
-//! disable the i2ctcp server
+//! Disable the TCP server
 #define CFG_SRV_DISABLE
 #endif
-/** \} */
+/// \}
 
 /**
  * \name I2C configuration
  * \{
  */
-#ifndef CFG_SDA_PIN
-//! I^2^C SDA pin
-#define CFG_SDA_PIN 16
+#ifndef CFG_SDA0_PIN
+//! I2C 0 SDA pin
+#define CFG_SDA0_PIN 16
+#endif
+#ifndef CFG_SCL0_PIN
+//! I2C 0 SCL pin
+#define CFG_SCL0_PIN 17
 #endif
-#ifndef CFG_SCL_PIN
-//! I^2^C SCL pin
-#define CFG_SCL_PIN 17
+#ifndef CFG_SDA1_PIN
+//! I2C 1 SDA pin
+#define CFG_SDA1_PIN 18
 #endif
-/** \} */
+#ifndef CFG_SCL1_PIN
+//! I2C 1 SCL pin
+#define CFG_SCL1_PIN 19
+#endif
+/// \}
 
+/**
+ * \name Auxiliary options
+ * \{
+ */
 #ifndef CFG_LED_PIN
-//! status LED pin
+//! Status LED pin
 #define CFG_LED_PIN CYW43_WL_GPIO_LED_PIN
 #endif
-
 #ifndef CFG_PB_MOD_MAX
-//! maximum number of simultaniously connected puzzle modules
+//! Maximum number of simultaniously connected puzzle modules
 #define CFG_PB_MOD_MAX 8
 #endif
+/// \}
+
+/// \}
 
diff --git a/main/i2c.c b/main/i2c.c
index 87ff204..d37a9b7 100644
--- a/main/i2c.c
+++ b/main/i2c.c
@@ -8,15 +8,31 @@
 
 #include "i2c.h"
 #include "pb-mod.h"
-#include "pbdrv.h"
 #include "config.h"
 #include "pb-buf.h"
 #include "pb-send.h"
 
 static pb_global_state_t _global_state = PB_GS_IDLE; 
 pb_puzzle_module_t modules[CFG_PB_MOD_MAX];
+// stolen from lib/pico-sdk/src/rp2_common/hardware_i2c/i2c.c
+#define i2c_reserved_addr(addr) (((addr) & 0x78) == 0 || ((addr) & 0x78) == 0x78)
 size_t modules_size = 0;
 
+static void bus_scan() {
+	pb_buf_t buf = pb_send_magic_req();
+
+	// check for all 7-bit addresses
+	uint16_t addr_max = 1 << 7;
+	for (uint16_t addr = 0x00; addr < addr_max; addr++) {
+		if (i2c_reserved_addr(addr)) continue;
+		if (addr == PB_MOD_ADDR) continue;
+
+		pb_i2c_send(addr, (uint8_t *) buf.data, buf.size);
+	}
+
+	pb_buf_free(&buf);
+}
+
 static void state_exchange() {
 	
 
@@ -74,19 +90,26 @@ void bus_task() {
 	// do a scan of the bus
 	bus_scan();
 
-	// FIXME: this should be removed (see handover: RP2040 I2C limitations)
-	// wait for 5 seconds until all handshake responses are received
-	pb_mod_blocking_delay_ms(5e3);
-
 	while(1) {
 		// send my state to all puzzle modules
 		state_exchange();
 
 		// wait 1 second
-		pb_mod_blocking_delay_ms(1e3);
+		vTaskDelay(1e3 / portTICK_PERIOD_MS);
 	}
 }
 
+/**
+ * \ingroup main_pb_override
+ * \anchor main_route_cmd_magic_res
+ *
+ * This function registers the I2C address of the puzzle module that replied to
+ * the \c MAGIC \c REQ command into a list of "known puzzle modules", which are
+ * then periodically updated during gameplay.
+ *
+ * \note Up to \ref CFG_PB_MOD_MAX puzzle modules can be registered
+ * simultaniously.
+ */
 void pb_route_cmd_magic_res(pb_msg_t * msg) {
 	if (modules_size == CFG_PB_MOD_MAX) return;
 	pb_puzzle_module_t tmp_module = {msg->sender, PB_GS_NOINIT};
diff --git a/main/i2c.h b/main/i2c.h
index d1173c8..27c0b02 100644
--- a/main/i2c.h
+++ b/main/i2c.h
@@ -1,5 +1,18 @@
 #pragma once
 
-//! looking for slave addresses and requesting updates
+/**
+ * \ingroup main_tasks
+ * \{
+ */
+
+/**
+ * \brief I2C bus activity task 
+ *
+ * This function does an initial bus scan for puzzle modules, and then goes
+ * into an infinite loop that periodically polls all puzzle modules for their
+ * global state.
+ */
 void bus_task();
 
+/// \}
+
diff --git a/main/index.dox b/main/index.dox
new file mode 100644
index 0000000..54c5b15
--- /dev/null
+++ b/main/index.dox
@@ -0,0 +1,13 @@
+// vim:ft=doxygen
+/**
+\ingroup main
+\defgroup main_tasks Tasks
+\brief FreeRTOS tasks
+*/
+
+/**
+\ingroup main
+\defgroup main_pb_override Overrides
+\brief Override functions from \ref pbdrv-mod
+*/
+
diff --git a/main/init.c b/main/init.c
index 6d29d19..25fa5e3 100644
--- a/main/init.c
+++ b/main/init.c
@@ -7,7 +7,7 @@
 #include "config.h"
 #include "init.h"
 #include "tasks.h"
-#include "pbdrv.h"
+#include "pb-mod.h"
 
 static void init_stdio() {
 	stdio_init_all();
@@ -29,11 +29,15 @@ static void init_wifi() {
 }
 
 static void init_i2c() {
-	gpio_set_function(CFG_SDA_PIN, GPIO_FUNC_I2C);
-	gpio_set_function(CFG_SCL_PIN, GPIO_FUNC_I2C);
+	gpio_set_function(CFG_SDA0_PIN, GPIO_FUNC_I2C);
+	gpio_set_function(CFG_SCL0_PIN, GPIO_FUNC_I2C);
+	gpio_set_function(CFG_SDA1_PIN, GPIO_FUNC_I2C);
+	gpio_set_function(CFG_SCL1_PIN, GPIO_FUNC_I2C);
 
-	gpio_pull_up(CFG_SDA_PIN);
-	gpio_pull_up(CFG_SCL_PIN);
+	gpio_pull_up(CFG_SDA0_PIN);
+	gpio_pull_up(CFG_SCL0_PIN);
+	gpio_pull_up(CFG_SDA1_PIN);
+	gpio_pull_up(CFG_SCL1_PIN);
 
 	pb_setup();
 }
diff --git a/main/init.h b/main/init.h
index 73d2773..a24977d 100644
--- a/main/init.h
+++ b/main/init.h
@@ -1,11 +1,17 @@
 #pragma once
 
 /**
- * \brief initialize the main controller
+ * \ingroup main
+ * \{
+ */
+
+/**
+ * \brief Initialize the main controller
  *
  * This function only synchronously initializes the standard input/output (used
- * for `printf` and `panic`), and queues all other types of initialization in
- * the `init` task using FreeRTOS.
+ * for \c printf() and \c panic()), and queues all other types of
+ * initialization in the \c async_init() task using FreeRTOS.
  */
 void init();
 
+/// \}
diff --git a/main/mod.c b/main/mod.c
index 11a1bb7..8650861 100644
--- a/main/mod.c
+++ b/main/mod.c
@@ -1,3 +1,6 @@
+#include <FreeRTOS.h>
+#include <task.h>
+
 #include "pb.h"
 #include "pb-mod.h"
 
diff --git a/main/pbdrv.c b/main/pbdrv.c
deleted file mode 100644
index 0624897..0000000
--- a/main/pbdrv.c
+++ /dev/null
@@ -1,95 +0,0 @@
-#include "pb.h"
-
-#include "pb.h"
-#include "pb-types.h"
-#include "pb-mod.h"
-#include "pb-send.h"
-
-#include <hardware/i2c.h>
-#include <hardware/gpio.h>
-#include <pico/i2c_slave.h>
-#include <pico/stdio.h>
-
-#include <FreeRTOS.h>
-#include <stdio.h>
-#include <timers.h>
-
-#define PB_I2C i2c0
-#define BUF_SIZE 256
-#define MSGS_MAX 4
-
-typedef struct {
-	uint8_t data[BUF_SIZE];
-	size_t size;
-} i2c_msg_buf_t;
-
-static i2c_msg_buf_t msgs[MSGS_MAX];
-static size_t msg_index = 0;
-
-static void async_pb_i2c_recv(void * _msg, uint32_t _) {
-	i2c_msg_buf_t * msg = _msg;
-	pb_i2c_recv(msg->data, msg->size);
-}
-
-static void msg_complete(i2c_msg_buf_t * msg) {
-	// defer pb_i2c_recv call to FreeRTOS scheduler as pb_i2c_recv takes
-	// too long to return from an ISR
-	xTimerPendFunctionCallFromISR(async_pb_i2c_recv, msg, 0, NULL);
-
-	// prepare next message for use
-	msg_index = (msg_index + 1) % MSGS_MAX;
-	msgs[msg_index].size = 0;
-}
-
-// This function is called from the I2C ISR
-static void recv_event(i2c_inst_t *i2c, i2c_slave_event_t event) {
-	i2c_msg_buf_t * msg = &msgs[msg_index];
-
-	switch (event) {
-		case I2C_SLAVE_RECEIVE: {
-			if (msg->size == BUF_SIZE) return;
-			msg->data[msg->size++] = i2c_read_byte_raw(PB_I2C);
-			break;
-		}
-		case I2C_SLAVE_FINISH: {
-			msg_complete(msg);
-			break;
-		}
-		default: break;
-	}
-}
-
-void pb_setup() {
-	i2c_init(PB_I2C, PB_CLOCK_SPEED_HZ);
-	i2c_slave_init(PB_I2C, PB_MOD_ADDR, &recv_event);
-}
-
-__weak void pb_i2c_send(i2c_addr_t addr, const uint8_t * buf, size_t sz) {
-	i2c_set_slave_mode(PB_I2C, false, PB_MOD_ADDR);
-
-	// false to write stop condition to i2c bus
-	i2c_write_timeout_us(PB_I2C, addr, buf, sz, false, PB_TIMEOUT_US);
-
-	i2c_set_slave_mode(PB_I2C, true, PB_MOD_ADDR);
-}
-
-void bus_scan() {
-	i2c_set_slave_mode(PB_I2C, false, PB_MOD_ADDR);
-
-	pb_buf_t buf = pb_send_magic_req();
-
-	// check for all 7-bit addresses
-	uint16_t addr_max = 1 << 7;
-	for (uint16_t addr = 0x00; addr < addr_max; addr++) {
-		i2c_write_timeout_us(PB_I2C, addr, (uint8_t *) buf.data, buf.size, false, PB_TIMEOUT_US);
-	}
-
-	pb_buf_free(&buf);
-
-	i2c_set_slave_mode(PB_I2C, true, PB_MOD_ADDR);
-}
-
-void pb_mod_blocking_delay_ms(unsigned long ms) {
-	vTaskDelay(ms / portTICK_PERIOD_MS);
-}
-
diff --git a/main/pbdrv.h b/main/pbdrv.h
deleted file mode 100644
index a751000..0000000
--- a/main/pbdrv.h
+++ /dev/null
@@ -1,52 +0,0 @@
-#pragma once
-
-#include "pb-types.h"
-
-/**
- * This is the RP2040 puzzle bus driver. This file is no longer inside
- * lib/pb//rp2040 as it is tightly coupled to both the pico-sdk and
- * freertos functions. I have tried to get FreeRTOS to play nicely with the
- * CMake subproject layout, but the pico-sdk and the rp2040 port of freertos
- * both rely on CMake's import() functionality, which makes using FreeRTOS in a
- * libary context extremely messy.
- *
- * The workaround implemented in this driver was already kind of messy, and a
- * different microcontroller should be used for the main controller instead.
- */
-
-#ifdef __cplusplus
-extern "C" {
-#endif
-
-//! puzzle bus driver setup
-void pb_setup();
-
-/**
- * While the RP2040's datasheet claims it supports multi-master configurations
- * by implementing bus arbitration, it does not natively support a mode where
- * it is configured as a (multi-)master with a slave address, such that it can
- * be addressed by other multi-masters. This function includes a hacky
- * workaround that teporarily sets the RP2040 to I2C master mode to send a
- * message, and then restores it back to slave mode.
- *
- * This approach results in some received frames being (partially) dropped in
- * the time period between the invocation of this function and the bus becoming
- * idle (and the message is sent).
- */
-void pb_i2c_send(i2c_addr_t addr, const uint8_t * buf, size_t sz);
-
-/**
- * \brief Scan the bus for I2C slaves, and send handshake messages to ACK-ing
- * slaves.
- *
- * As a result of the RP2040 hardware limitations detailed at the top of this
- * file, this function is also implemented in this file, even through it does
- * not belong to the puzzle bus driver. In order to not miss any handshake
- * responses, the bus should remain busy during the entire scan.
- */
-void bus_scan();
-
-#ifdef __cplusplus
-}
-#endif
-
diff --git a/main/readme.md b/main/readme.md
index 28fcfad..97150eb 100644
--- a/main/readme.md
+++ b/main/readme.md
@@ -1,3 +1,6 @@
+\defgroup main main
+\brief Main controller software
+
 # main controller firmware
 
 This directory contains the software for the main controller of the Puzzle Box.
@@ -5,17 +8,7 @@ This directory contains the software for the main controller of the Puzzle Box.
 ## building
 
 1. make sure the submodules are initialized
-2. create a `config.h` file and define some options (see `config.def.h` for all
-   options):
-   ```c
-   #pragma once
-
-   #define CFG_NET_SSID "network name"
-   #define CFG_NET_PASS "network password"
-   #define CFG_NET_AUTH CYW43_AUTH_WPA2_AES_PSK
-
-   #include "config.def.h"
-   ```
+2. create a `config.h` file (see \ref main_config "config")
 3. use CMake to build
 
 ## flashing
diff --git a/main/sock.h b/main/sock.h
index 38fca01..a151973 100644
--- a/main/sock.h
+++ b/main/sock.h
@@ -1,8 +1,17 @@
 #pragma once
 
-#include <stdint.h>
-#include <stddef.h>
+/**
+ * \ingroup main_tasks
+ * \{
+ */
 
-//! start listening for TCP socket requests
+/**
+ * \brief Listen for TCP socket messages
+ *
+ * This task starts a TCP server that listens for messages using \ref i2ctcp,
+ * and sends any received I2C messages (also using \ref i2ctcp).
+ */
 void serve_task();
 
+/// \}
+
diff --git a/main/tasks.h b/main/tasks.h
index 002f830..ac77a9e 100644
--- a/main/tasks.h
+++ b/main/tasks.h
@@ -1,4 +1,13 @@
 #pragma once
 
+/**
+ * \ingroup main
+ * \{
+ */
+
+/**
+ * \brief Register \ref main_tasks "all tasks" in FreeRTOS
+ */
 void init_tasks();
 
+/// \}