From bb63040692c94ffa662b0af7eb14f3c5951aa6e6 Mon Sep 17 00:00:00 2001
From: Loek Le Blansch <loek@pipeframe.xyz>
Date: Sat, 22 Jun 2024 13:01:42 +0200
Subject: even more doxygen documentation

---
 main/pbdrv.h | 56 ++++++++++++++++++++++++++++++++++----------------------
 1 file changed, 34 insertions(+), 22 deletions(-)

(limited to 'main')

diff --git a/main/pbdrv.h b/main/pbdrv.h
index a751000..9496aa9 100644
--- a/main/pbdrv.h
+++ b/main/pbdrv.h
@@ -3,35 +3,41 @@
 #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.
+ * \ingroup pb_drv
+ * \defgroup pb_drv_rp2040 RP2040
+ * \brief Raspberry Pi Pico and Pico W driver
  *
- * The workaround implemented in this driver was already kind of messy, and a
- * different microcontroller should be used for the main controller instead.
+ * \note This file is no longer inside `lib/pbdrv/drv/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.
+ *
+ * \warning The workaround implemented in this driver was already kind of
+ * messy, and **a different microcontroller should be used for the main
+ * controller instead**. Please see the handover document for more details.
+ *
+ * \{
  */
 
 #ifdef __cplusplus
 extern "C" {
 #endif
 
-//! puzzle bus driver setup
+//! 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.
+ * \note 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).
+ * \warning 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);
 
@@ -39,13 +45,19 @@ 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.
+ * \note As a result of the RP2040 hardware limitations, this function is also
+ * implemented in this file, even though it does not belong to the puzzle bus
+ * driver.
+ *
+ * \warning In order to not miss any handshake responses, the bus should remain
+ * busy during the entire scan. The \c nostop parameter of the \c
+ * i2c_write_timeout_us() function from the pico-sdk does not seem to keep the
+ * bus busy.
  */
 void bus_scan();
 
+/// \}
+
 #ifdef __cplusplus
 }
 #endif
-- 
cgit v1.2.3


From 33d17dbab86c1277a1327273fb9753b2c98b7b03 Mon Sep 17 00:00:00 2001
From: Loek Le Blansch <loek@pipeframe.xyz>
Date: Sat, 22 Jun 2024 14:32:19 +0200
Subject: more documentation

---
 Doxyfile               |  6 ++++--
 client/readme.md       | 32 ++++++++++++++------------------
 lib/i2ctcp/i2ctcpv1.h  |  2 ++
 lib/mpack/mpack.h      | 31 ++++++++++++++++++++++++++-----
 main/readme.md         |  3 +++
 makefile               |  9 ++++++++-
 puzzle/dummy/index.dox |  9 +++++++++
 puzzle/neo/index.dox   |  6 ++++++
 puzzle/readme.md       | 17 +++++++++++------
 puzzle/vault/index.dox |  6 ++++++
 readme.md              | 22 ++++++++++++++++++++--
 11 files changed, 109 insertions(+), 34 deletions(-)
 create mode 100644 puzzle/dummy/index.dox
 create mode 100644 puzzle/neo/index.dox
 create mode 100644 puzzle/vault/index.dox

(limited to 'main')

diff --git a/Doxyfile b/Doxyfile
index 2d600e7..af362dd 100644
--- a/Doxyfile
+++ b/Doxyfile
@@ -4,12 +4,12 @@ PROJECT_NAME = "puzzlebox"
 OUTPUT_DIRECTORY = doxygen
 
 INPUT += readme.md
-INPUT += client
 INPUT += lib/mpack
 INPUT += lib/i2ctcp
 INPUT += lib/pbdrv
+INPUT += client
 INPUT += main
-INPUT += puzzle/dummy
+INPUT += puzzle
 
 EXCLUDE = lib/mpack/src
 EXCLUDE_PATTERNS = **/build
@@ -24,6 +24,8 @@ ALIASES += I2C="I&sup2;C"
 INPUT_FILTER = "sed -e 's/\<I2C\>\|\<I<sup>2<\/sup>C\>/\\I2C/g'"
 
 USE_MDFILE_AS_MAINPAGE = readme.md
+HTML_INDEX_NUM_ENTRIES = 1 # collapse trees by default
+REPEAT_BRIEF = NO
 
 INTERNAL_DOCS = YES
 EXTRACT_STATIC = YES
diff --git a/client/readme.md b/client/readme.md
index fcde40d..b9e0b09 100644
--- a/client/readme.md
+++ b/client/readme.md
@@ -1,3 +1,6 @@
+\defgroup pbc pbc
+\brief Puzzle box client
+
 # puzzle box client
 
 This folder contains the source code for the puzzle box client (pbc). This is a
@@ -7,30 +10,21 @@ game operator to control and monitor the state of a puzzle box, but is also a
 useful debugging tool when developing puzzle modules, as it allows you to send
 arbitrary data over the puzzle bus.
 
-## WIP TODO
-
-- cleanup
-  - separate ../shared/pb/moddrv.c into a puzzle module specific and 'common' bit
-  - use the common bit in i2c.cpp instead
-  - cast to structs in ../shared/pb/moddrv.c
-- functionality
-  - print pretty tree of connected puzzle modules
-  - add enum to string functions in CLIENT ONLY
-
 ## Features
 
 - List detected puzzle modules
 - Reset puzzle modules (individually or all to reset the box)
 - Skip puzzle modules (individually or all)
 - Request puzzle box state
-
-Debug only:
-- Send arbitrary messages
+- Debug: send arbitrary messages
 
 ## Building
 
-PBC is a standard CMake project, but a [makefile](./makefile) is provided for
-convenience (still requires CMake and Ninja are installed).
+PBC is a standard CMake project.
+
+## Using
+
+See ./pbc.1 for usage.
 
 ## Send data
 
@@ -45,8 +39,10 @@ send 0x39     68:65:6c:6c:6f  44      0x20    'world'    33
 
 The data is concatenated, and may contain mixed types of literals
 
-## known bugs (TODO)
 
-- tab completion for `dump` seems to print garbage sometimes
-- the send command with an address but no data causes a segmentation fault
+## WIP TODO
+
+- add enum to string functions in CLIENT ONLY
+- bug: tab completion for `dump` seems to print garbage sometimes
+- bug: the send command with an address but no data causes a segmentation fault
 
diff --git a/lib/i2ctcp/i2ctcpv1.h b/lib/i2ctcp/i2ctcpv1.h
index d7bb29e..e9bc0d9 100644
--- a/lib/i2ctcp/i2ctcpv1.h
+++ b/lib/i2ctcp/i2ctcpv1.h
@@ -91,6 +91,8 @@ void i2ctcp_read_reset(i2ctcp_msg_t * target);
  */
 bool i2ctcp_write(const i2ctcp_msg_t * target, char ** buf, size_t * buf_sz);
 
+/// \}
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/lib/mpack/mpack.h b/lib/mpack/mpack.h
index 7c0c089..33521c4 100644
--- a/lib/mpack/mpack.h
+++ b/lib/mpack/mpack.h
@@ -2,18 +2,39 @@
 
 #include "src/src/mpack/mpack.h"
 
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/**
+ * \defgroup mpack mpack
+ * \brief Mpack extensions specific to this project
+ *
+ * The mpack folder under `lib/` contains a CMakeLists.txt for integrating the
+ * mapck library with CMake, and includes an extension in the form of an
+ * additional utility function.
+ *
+ * \{
+ */
+
 /**
- * \brief read remaining bytes in reader without opening a tag first
+ * \brief Read remaining bytes in reader without opening a tag first
  *
- * \param reader  pointer to mpack reader object
- * \param p  pointer to write data to
- * \param count  maximum number of bytes to read
+ * \param reader  Pointer to mpack reader object
+ * \param p  Pointer to write data to
+ * \param count  Maximum number of bytes to read
  *
  * This function reads *up to* the amount of bytes specified in \p count, or
  * less if there is less remaining data in the buffer. If \p count is equal to
  * 0, all remaining data in the buffer is read.
  *
- * \return amount of bytes read
+ * \return Amount of bytes read
  */
 size_t mpack_read_remaining_bytes(mpack_reader_t * reader, char * p, size_t count);
 
+/// \}
+
+#ifdef __cplusplus
+}
+#endif
+
diff --git a/main/readme.md b/main/readme.md
index 28fcfad..85a3fca 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.
diff --git a/makefile b/makefile
index 5f5340b..090877e 100644
--- a/makefile
+++ b/makefile
@@ -1,5 +1,12 @@
 all: doxygen
 
-doxygen: Doxyfile
+.PHONY: FORCE
+
+doxygen: Doxyfile FORCE
 	doxygen
 
+FMT += $(shell git ls-files '*.h' '*.c' '*.cpp')
+format: FORCE
+	clang-format -i $(FMT)
+	clang-tidy --fix-errors $(FMT)
+
diff --git a/puzzle/dummy/index.dox b/puzzle/dummy/index.dox
new file mode 100644
index 0000000..918bf7f
--- /dev/null
+++ b/puzzle/dummy/index.dox
@@ -0,0 +1,9 @@
+// vim:ft=doxygen
+/**
+\ingroup puz
+\defgroup puz_dummy Dummy
+\brief Demo puzzle module
+
+The dummy puzzle module consists of an empty Arduino sketch, and can be used as
+a starting point for developing new puzzle modules.
+*/
diff --git a/puzzle/neo/index.dox b/puzzle/neo/index.dox
new file mode 100644
index 0000000..14cefdc
--- /dev/null
+++ b/puzzle/neo/index.dox
@@ -0,0 +1,6 @@
+// vim:ft=doxygen
+/**
+\ingroup puz
+\defgroup puz_neo Neo
+\brief NeoTrellis puzzle module
+*/
diff --git a/puzzle/readme.md b/puzzle/readme.md
index 59c10b1..959c506 100644
--- a/puzzle/readme.md
+++ b/puzzle/readme.md
@@ -1,15 +1,20 @@
+\defgroup puz puzzle
+\brief Puzzle modules
+
 # puzzles
 
 This folder contains the source code for all puzzle modules.
 
 ## Arduino-based puzzle modules
 
-Because of the poorly designed hardware (21-22) used during development
-(23-24), some puzzle modules ended up being developed using Arduino boards. All
-libraries in this repository use CMake for building (for consistency), which
-also means the Arduino based puzzle modules use CMake. The CMakeLists.txt of
-some puzzles uses the [Arduino-CMake-Toolchain][arduino-cmake]. To build any of
-these subfolders, make sure you have done the following:
+> [!NOTE]
+> Because of the poorly designed hardware (21-22) used during development
+> (23-24), all puzzle modules ended up being developed using Arduino boards.
+
+All libraries in this repository use CMake for building (for consistency),
+which also means the Arduino based puzzle modules use CMake. The CMakeLists.txt
+of some puzzles uses the [Arduino-CMake-Toolchain][arduino-cmake]. To build any
+of these subfolders, make sure you have done the following:
 
 - Install the official Arduino IDE
 - Open "Tools" > "Board" > "Board manager"
diff --git a/puzzle/vault/index.dox b/puzzle/vault/index.dox
new file mode 100644
index 0000000..b14e616
--- /dev/null
+++ b/puzzle/vault/index.dox
@@ -0,0 +1,6 @@
+// vim:ft=doxygen
+/**
+\ingroup puz
+\defgroup puz_vault Vault
+\brief Vault puzzle module
+*/
diff --git a/readme.md b/readme.md
index 457ffaa..a6365c0 100644
--- a/readme.md
+++ b/readme.md
@@ -11,6 +11,20 @@ project. The software in this repository should be easily portable to various
 other microcontrollers, and a recommendation is made in the [design
 document](docs/design.adoc).
 
+## Documentation
+
+This project is documented using Doxygen. To generate HTML docs, run
+
+```
+$ make doxygen
+```
+
+and open <doxygen/html/index.html> in a browser.
+
+If you are viewing this page from Doxygen, please take a look at
+[topics](/topics.html) for a comprehensive list of components within this
+project.
+
 ## Tidyness
 
 Please keep this repository tidy by being aware of the following conventions!
@@ -36,8 +50,12 @@ An `.editorconfig` file is provided in this repository. Please install the
 [EditorConfig](https://editorconfig.org/) plugin for your text editor of choice
 to automatically use these.
 
-Currently, no linter/formatter is configured for maintaining consistent code
-style.
+There are also `.clang-tidy` and `.clang-format` files, which can be enforced
+by running
+
+```
+$ make format
+```
 
 ## Submodules
 
-- 
cgit v1.2.3


From 3c2f4422e4dd90826a549c87d9d042c592806cb8 Mon Sep 17 00:00:00 2001
From: Loek Le Blansch <loek@pipeframe.xyz>
Date: Sat, 22 Jun 2024 17:26:17 +0200
Subject: docs done

---
 main/blink.h      | 12 ++++++++++++
 main/config.def.h | 54 ++++++++++++++++++++++++++++++++++++------------------
 main/i2c.h        | 14 +++++++++++++-
 main/index.dox    |  6 ++++++
 main/init.h       | 12 +++++++++---
 main/sock.h       | 15 ++++++++++++---
 main/tasks.h      |  9 +++++++++
 7 files changed, 97 insertions(+), 25 deletions(-)
 create mode 100644 main/index.dox

(limited to 'main')

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..0dae608 100644
--- a/main/config.def.h
+++ b/main/config.def.h
@@ -2,32 +2,45 @@
 #include <pico/cyw43_arch.h>
 #include <cyw43_country.h>
 
+/**
+ * \ingroup main
+ * \defgroup main_config config
+ * \brief Configuration options
+ * \{
+ */
+
 /**
  * \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 +49,52 @@
 #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
+//! I2C SDA pin
 #define CFG_SDA_PIN 16
 #endif
 #ifndef CFG_SCL_PIN
-//! I^2^C SCL pin
+//! I2C SCL pin
 #define CFG_SCL_PIN 17
 #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.h b/main/i2c.h
index d1173c8..107a04d 100644
--- a/main/i2c.h
+++ b/main/i2c.h
@@ -1,5 +1,17 @@
 #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..aa2d07a
--- /dev/null
+++ b/main/index.dox
@@ -0,0 +1,6 @@
+// vim:ft=doxygen
+/**
+\ingroup main
+\defgroup main_tasks tasks
+\brief Tasks
+*/
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/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();
 
+/// \}
-- 
cgit v1.2.3


From ccdcf5001c47820c41e9b962d8498602870289b5 Mon Sep 17 00:00:00 2001
From: Loek Le Blansch <loek@pipeframe.xyz>
Date: Sat, 22 Jun 2024 18:32:06 +0200
Subject: WIP RP2040 I2C workaround code

---
 lib/pbdrv/pb-route.c |  3 ---
 main/config.def.h    | 20 ++++++++++++++------
 main/i2c.c           | 16 ++++++++++++----
 main/init.c          | 12 ++++++++----
 main/pbdrv.c         | 36 +++++++++++-------------------------
 main/pbdrv.h         | 15 ---------------
 6 files changed, 45 insertions(+), 57 deletions(-)

(limited to 'main')

diff --git a/lib/pbdrv/pb-route.c b/lib/pbdrv/pb-route.c
index f5c32d6..5a7bd67 100644
--- a/lib/pbdrv/pb-route.c
+++ b/lib/pbdrv/pb-route.c
@@ -93,9 +93,6 @@ __weak void pb_route_cmd_magic_req(pb_msg_t * msg) {
 	// // return early if magic doesn't match
 	if (pb_memcmp(cmd->magic, pb_cmd_magic_req, sizeof(pb_cmd_magic_req)) != 0) return;
 
-	// FIXME: this should be removed (see handover: RP2040 I2C limitations)
-	pb_mod_blocking_delay_ms(2000);
-
 	pb_buf_t buf = pb_send_magic_res();
 	pb_send_reply(msg, &buf);
 	pb_buf_free(&buf);
diff --git a/main/config.def.h b/main/config.def.h
index 0dae608..b3be5ed 100644
--- a/main/config.def.h
+++ b/main/config.def.h
@@ -72,13 +72,21 @@
  * \name I2C configuration
  * \{
  */
-#ifndef CFG_SDA_PIN
-//! I2C SDA pin
-#define CFG_SDA_PIN 16
+#ifndef CFG_SDA0_PIN
+//! I2C 0 SDA pin
+#define CFG_SDA0_PIN 16
 #endif
-#ifndef CFG_SCL_PIN
-//! I2C SCL pin
-#define CFG_SCL_PIN 17
+#ifndef CFG_SCL0_PIN
+//! I2C 0 SCL pin
+#define CFG_SCL0_PIN 17
+#endif
+#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
 /// \}
 
diff --git a/main/i2c.c b/main/i2c.c
index 2503560..0f23a13 100644
--- a/main/i2c.c
+++ b/main/i2c.c
@@ -16,6 +16,18 @@
 i2c_addr_t modules[CFG_PB_MOD_MAX];
 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++) {
+		pb_i2c_send(addr, (uint8_t *) buf.data, buf.size);
+	}
+
+	pb_buf_free(&buf);
+}
+
 static void state_exchange() {
 	for (size_t i = 0; i < modules_size; i++) {
 		pb_buf_t buf = pb_send_state_req();
@@ -28,10 +40,6 @@ 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();
diff --git a/main/init.c b/main/init.c
index 6d29d19..d85c94d 100644
--- a/main/init.c
+++ b/main/init.c
@@ -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/pbdrv.c b/main/pbdrv.c
index 0624897..6a89253 100644
--- a/main/pbdrv.c
+++ b/main/pbdrv.c
@@ -14,7 +14,9 @@
 #include <stdio.h>
 #include <timers.h>
 
-#define PB_I2C i2c0
+#define PB_I2C_S i2c0
+#define PB_I2C_M i2c1
+
 #define BUF_SIZE 256
 #define MSGS_MAX 4
 
@@ -32,6 +34,8 @@ static void async_pb_i2c_recv(void * _msg, uint32_t _) {
 }
 
 static void msg_complete(i2c_msg_buf_t * msg) {
+	return pb_i2c_recv(msg->data, msg->size);
+
 	// 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);
@@ -48,7 +52,7 @@ static void recv_event(i2c_inst_t *i2c, i2c_slave_event_t event) {
 	switch (event) {
 		case I2C_SLAVE_RECEIVE: {
 			if (msg->size == BUF_SIZE) return;
-			msg->data[msg->size++] = i2c_read_byte_raw(PB_I2C);
+			msg->data[msg->size++] = i2c_read_byte_raw(PB_I2C_S);
 			break;
 		}
 		case I2C_SLAVE_FINISH: {
@@ -60,33 +64,15 @@ static void recv_event(i2c_inst_t *i2c, i2c_slave_event_t event) {
 }
 
 void pb_setup() {
-	i2c_init(PB_I2C, PB_CLOCK_SPEED_HZ);
-	i2c_slave_init(PB_I2C, PB_MOD_ADDR, &recv_event);
+	i2c_init(PB_I2C_S, PB_CLOCK_SPEED_HZ);
+	i2c_init(PB_I2C_M, PB_CLOCK_SPEED_HZ);
+
+	i2c_slave_init(PB_I2C_S, 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);
+	i2c_write_timeout_us(PB_I2C_M, addr, buf, sz, false, PB_TIMEOUT_US);
 }
 
 void pb_mod_blocking_delay_ms(unsigned long ms) {
diff --git a/main/pbdrv.h b/main/pbdrv.h
index 9496aa9..89a4870 100644
--- a/main/pbdrv.h
+++ b/main/pbdrv.h
@@ -41,21 +41,6 @@ void pb_setup();
  */
 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.
- *
- * \note As a result of the RP2040 hardware limitations, this function is also
- * implemented in this file, even though it does not belong to the puzzle bus
- * driver.
- *
- * \warning In order to not miss any handshake responses, the bus should remain
- * busy during the entire scan. The \c nostop parameter of the \c
- * i2c_write_timeout_us() function from the pico-sdk does not seem to keep the
- * bus busy.
- */
-void bus_scan();
-
 /// \}
 
 #ifdef __cplusplus
-- 
cgit v1.2.3


From 648d87ea98ec39d5745d36a0b5c5078cd9491211 Mon Sep 17 00:00:00 2001
From: Loek Le Blansch <loek@pipeframe.xyz>
Date: Sun, 23 Jun 2024 13:31:49 +0200
Subject: place rp2040 back in pbdrv

---
 lib/pbdrv/CMakeLists.txt             |  6 +--
 lib/pbdrv/drv/arduino/include.cmake  |  6 +--
 lib/pbdrv/drv/arduino/index.dox      | 20 +++++----
 lib/pbdrv/drv/arduino/mod.cpp        | 42 ++++++-------------
 lib/pbdrv/drv/index.dox              | 13 ------
 lib/pbdrv/drv/rp2040/include.cmake   | 11 +++++
 lib/pbdrv/drv/rp2040/index.dox       | 31 ++++++++++++++
 lib/pbdrv/drv/rp2040/mod.c           | 49 ++++++++++++++++++++++
 lib/pbdrv/drv/rp2040/pb-mod.h        | 21 ++++++++++
 lib/pbdrv/ext/freertos/include.cmake |  2 +-
 lib/pbdrv/index.dox                  | 13 +++---
 main/CMakeLists.txt                  |  4 +-
 main/i2c.c                           |  1 -
 main/init.c                          |  2 +-
 main/mod.c                           |  7 ++++
 main/pbdrv.c                         | 81 ------------------------------------
 main/pbdrv.h                         | 49 ----------------------
 puzzle/dummy/CMakeLists.txt          |  3 +-
 18 files changed, 157 insertions(+), 204 deletions(-)
 create mode 100644 lib/pbdrv/drv/rp2040/include.cmake
 create mode 100644 lib/pbdrv/drv/rp2040/index.dox
 create mode 100644 lib/pbdrv/drv/rp2040/mod.c
 create mode 100644 lib/pbdrv/drv/rp2040/pb-mod.h
 delete mode 100644 main/pbdrv.c
 delete mode 100644 main/pbdrv.h

(limited to 'main')

diff --git a/lib/pbdrv/CMakeLists.txt b/lib/pbdrv/CMakeLists.txt
index 998ed4d..903a3b4 100644
--- a/lib/pbdrv/CMakeLists.txt
+++ b/lib/pbdrv/CMakeLists.txt
@@ -23,7 +23,7 @@ add_library(pbdrv STATIC
 	pb-buf.c
 	)
 target_include_directories(pbdrv SYSTEM INTERFACE .)
-target_link_libraries(pbdrv mpack)
+target_link_libraries(pbdrv PRIVATE mpack)
 
 # puzzle bus *module* specific code
 add_library(pbdrv-mod OBJECT
@@ -32,9 +32,9 @@ add_library(pbdrv-mod OBJECT
 	pb-route.c
 	)
 target_include_directories(pbdrv-mod SYSTEM INTERFACE .)
-target_link_libraries(pbdrv-mod pbdrv)
+target_link_libraries(pbdrv-mod PUBLIC pbdrv)
 
 # puzzle bus drivers
 include(drv/arduino/include.cmake)
-# include(drv/rp2040/include.cmake) # please see /main/pbdrv.h
+include(drv/rp2040/include.cmake)
 
diff --git a/lib/pbdrv/drv/arduino/include.cmake b/lib/pbdrv/drv/arduino/include.cmake
index 1e2ff08..520bfc1 100644
--- a/lib/pbdrv/drv/arduino/include.cmake
+++ b/lib/pbdrv/drv/arduino/include.cmake
@@ -3,9 +3,5 @@ if(NOT DEFINED ARDUINO)
 endif()
 
 target_sources(pbdrv-mod PRIVATE "${CMAKE_CURRENT_LIST_DIR}/mod.cpp")
-target_link_arduino_libraries(pbdrv-mod core Wire)
-
-# freertos is used to defer the handling of i2c messages outside the receive
-# interrupt service routine
-include("${CMAKE_CURRENT_LIST_DIR}/../../ext/freertos/include.cmake")
+target_link_arduino_libraries(pbdrv-mod PRIVATE core Wire)
 
diff --git a/lib/pbdrv/drv/arduino/index.dox b/lib/pbdrv/drv/arduino/index.dox
index 4c74222..1856918 100644
--- a/lib/pbdrv/drv/arduino/index.dox
+++ b/lib/pbdrv/drv/arduino/index.dox
@@ -2,18 +2,22 @@
 /**
 \ingroup pb_drv
 \defgroup pb_drv_arduino Arduino
-\brief Arduino (Arduino-CMake-Toolchain) driver
-
-This driver is automatically enabled if the variable \c ARDUINO is defined in
-your CMakeLists.txt (it is by default when using Arduino-CMake-Toolchain).
-
-\note This driver automatically includes the
-\ref pb_ext_freertos "FreeRTOS extension" for deferring calls to \c
-pb_i2c_recv() from the I2C ISR.
+\brief Arduino ATmega (w/ Arduino-CMake-Toolchain) driver
 
 This driver is known to work with the following MCUs:
 - ATmega328P (Arduino Uno)
 - ATmega2560 (Arduino Mega)
 
+\par Usage
+- Link the \c pbdrv-mod library with your main executable
+- Load an \ref pb_ext "extension"
+
+\note This driver is automatically enabled if the variable \c ARDUINO is
+defined in your CMakeLists.txt (it is by default when using
+Arduino-CMake-Toolchain).
+
+A complete example for this driver is available in the \ref puzzle/dummy
+folder.
+
 */
 
diff --git a/lib/pbdrv/drv/arduino/mod.cpp b/lib/pbdrv/drv/arduino/mod.cpp
index 2eef8d5..d2a6402 100644
--- a/lib/pbdrv/drv/arduino/mod.cpp
+++ b/lib/pbdrv/drv/arduino/mod.cpp
@@ -6,32 +6,17 @@
 #include <Wire.h>
 #include <avr/delay.h>
 
-#include <FreeRTOS.h>
-#include <timers.h>
-#include <task.h>
-
 #include "../../pb.h"
 #include "../../pb-mod.h"
 #include "../../pb-types.h"
-#include "../../pb-buf.h"
-#include "../../pb-mem.h"
-
-static void async_pb_i2c_recv(void * _msg, uint32_t _) {
-	pb_buf_t * msg = (pb_buf_t *) _msg;
-	pb_i2c_recv((uint8_t *) msg->data, msg->size);
-	pb_buf_free(msg);
-	pb_free(msg);
-}
 
 static void recv_event(int bytes) {
-	pb_buf_t * msg = (pb_buf_t *) pb_malloc(sizeof(pb_buf_t));
-	msg->data = (char *) pb_malloc(bytes);
-	msg->size = 0;
+	uint8_t data[bytes];
+	size_t sz = 0;
 	while (Wire.available())
-		msg->data[msg->size++] = Wire.read();
+		data[sz++] = Wire.read();
 
-	// defer pb_i2c_recv call
-	xTimerPendFunctionCallFromISR(async_pb_i2c_recv, msg, 0, NULL);
+	pb_i2c_recv(data, sz);
 }
 
 static void pb_setup() {
@@ -42,7 +27,6 @@ static void pb_setup() {
 	Wire.onReceive(recv_event);
 }
 
-/// \ingroup pb_drv_arduino
 __weak void pb_i2c_send(i2c_addr_t addr, const uint8_t * buf, size_t sz) {
 	Wire.beginTransmission((int) addr);
 	Wire.write(buf, sz);
@@ -57,18 +41,14 @@ extern void loop(void);
 //! Arduino internal initialization
 void init(void);
 
-//! FreeRTOS loop task
-void loop_task() {
-	for(;;) {
-		loop();
-		if (serialEventRun) serialEventRun();
-	}
-}
-
 /**
  * \ingroup pb_drv_arduino
  * \brief Application entrypoint
  *
+ * This function overrides the default (weak) implementation of the \c main()
+ * function in the Arduino framework. No additional setup is required to use
+ * this driver.
+ *
  * \note I should really be able to use Arduino's initVariant function for
  * this, but I can't seem to get it to link properly using the CMake setup in
  * this repository. Overriding the main() function seems to work, and the
@@ -79,8 +59,10 @@ int main(void) {
 	init(); // call arduino internal setup
 	setup(); // call regular arduino setup
 	pb_setup(); // call pbdrv-mod setup
-	xTaskCreate((TaskFunction_t) loop_task, "loop", configMINIMAL_STACK_SIZE, NULL, tskIDLE_PRIORITY + 1, NULL);
-	vTaskStartScheduler(); // start freertos scheduler
+	for(;;) {
+		loop();
+		if (serialEventRun) serialEventRun();
+	}
 	return 0;
 }
 
diff --git a/lib/pbdrv/drv/index.dox b/lib/pbdrv/drv/index.dox
index 1fe09e2..89b9247 100644
--- a/lib/pbdrv/drv/index.dox
+++ b/lib/pbdrv/drv/index.dox
@@ -7,19 +7,6 @@
 Like \ref pb_ext "extensions", drivers provide platform-specific
 implementations for various functions used in \ref pbdrv-mod.
 
-Drivers are automatically included based on your build configuration, and you
-only need to ensure \c pbdrv-mod is linked with your final executable in order
-to use one of the available drivers:
-
-```cmake
-# include pbdrv
-add_subdirectory(lib/pbdrv)
-
-# link pbdrv-mod
-target_link_libraries(main pbdrv-mod)
-
-```
-
 If there is no existing driver for your target, you may implement the following
 in order to use \ref pbdrv-mod:
 
diff --git a/lib/pbdrv/drv/rp2040/include.cmake b/lib/pbdrv/drv/rp2040/include.cmake
new file mode 100644
index 0000000..6d25e96
--- /dev/null
+++ b/lib/pbdrv/drv/rp2040/include.cmake
@@ -0,0 +1,11 @@
+if(NOT PICO_PLATFORM STREQUAL "rp2040")
+	return()
+endif()
+
+target_sources(pbdrv-mod PRIVATE "${CMAKE_CURRENT_LIST_DIR}/mod.c")
+target_link_libraries(pbdrv-mod PRIVATE hardware_i2c_headers pico_i2c_slave_headers)
+target_include_directories(pbdrv-mod INTERFACE "${CMAKE_CURRENT_LIST_DIR}")
+
+# pico-stdlib is compatible with C stdlib
+include("${CMAKE_CURRENT_LIST_DIR}/../../ext/stdlib/include.cmake")
+
diff --git a/lib/pbdrv/drv/rp2040/index.dox b/lib/pbdrv/drv/rp2040/index.dox
new file mode 100644
index 0000000..eb01d15
--- /dev/null
+++ b/lib/pbdrv/drv/rp2040/index.dox
@@ -0,0 +1,31 @@
+// vim:ft=doxygen
+/**
+\ingroup pb_drv
+\defgroup pb_drv_rp2040 RP2040
+\brief Raspberry Pi Pico and Pico W driver
+
+This driver is known to work with the following MCUs:
+- Raspberry Pi Pico W
+
+\note 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.
+
+\note This driver includes a workaround that uses both I2C peripherals on the
+RP2040. To use this driver, make sure you initialize both I2C peripherals, and
+connect their respective SDA and SCL pins to the same I2C bus.
+
+\par Usage
+- Link the \c pbdrv-mod library with your main executable
+- Call the \c pb_setup() function (from \c <pb-mod.h>) during setup
+
+\note This driver is automatically enabled if the variable \c PICO_PLATFORM is
+equal to \c "rp2040" in your CMakeLists.txt (it is by default when using the
+pico-sdk CMake library).
+
+\note This driver automatically includes the \ref pb_ext_stdlib
+"stdlib extension" as the pico-sdk implements the C standard library.
+
+*/
+
diff --git a/lib/pbdrv/drv/rp2040/mod.c b/lib/pbdrv/drv/rp2040/mod.c
new file mode 100644
index 0000000..1535da9
--- /dev/null
+++ b/lib/pbdrv/drv/rp2040/mod.c
@@ -0,0 +1,49 @@
+#include "pb.h"
+
+#include "pb.h"
+#include "pb-types.h"
+#include "pb-mod.h"
+#include "pb-send.h"
+#include "pb-buf.h"
+
+#include <hardware/i2c.h>
+#include <hardware/gpio.h>
+#include <pico/i2c_slave.h>
+
+#define PB_I2C_S i2c0
+#define PB_I2C_M i2c1
+
+#define BUF_SIZE 256
+
+uint8_t i2c_msg_buf[BUF_SIZE];
+size_t i2c_msg_buf_sz = 0;
+
+// This function is called from the I2C ISR
+static void recv_event(i2c_inst_t *i2c, i2c_slave_event_t event) {
+	switch (event) {
+		case I2C_SLAVE_RECEIVE: {
+			if (i2c_msg_buf_sz == BUF_SIZE) return;
+			i2c_msg_buf[i2c_msg_buf_sz++] = i2c_read_byte_raw(PB_I2C_S);
+			break;
+		}
+		case I2C_SLAVE_FINISH: {
+			pb_i2c_recv(i2c_msg_buf, i2c_msg_buf_sz);
+			i2c_msg_buf_sz = 0;
+			break;
+		}
+		default: break;
+	}
+}
+
+void pb_setup() {
+	i2c_init(PB_I2C_S, PB_CLOCK_SPEED_HZ);
+	i2c_init(PB_I2C_M, PB_CLOCK_SPEED_HZ);
+
+	i2c_slave_init(PB_I2C_S, PB_MOD_ADDR, &recv_event);
+}
+
+__weak void pb_i2c_send(i2c_addr_t addr, const uint8_t * buf, size_t sz) {
+	// false to write stop condition to i2c bus
+	i2c_write_timeout_us(PB_I2C_M, addr, buf, sz, false, PB_TIMEOUT_US);
+}
+
diff --git a/lib/pbdrv/drv/rp2040/pb-mod.h b/lib/pbdrv/drv/rp2040/pb-mod.h
new file mode 100644
index 0000000..96edc70
--- /dev/null
+++ b/lib/pbdrv/drv/rp2040/pb-mod.h
@@ -0,0 +1,21 @@
+#pragma once
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/**
+ * \ingroup pb_drv_rp2040
+ * \brief Puzzle bus driver setup
+ *
+ * This function must be called from a setup/init function to initialize the
+ * I2C peripherals used by this driver.
+ */
+void pb_setup();
+
+#ifdef __cplusplus
+}
+#endif
+
+#include "../../pb-mod.h"
+
diff --git a/lib/pbdrv/ext/freertos/include.cmake b/lib/pbdrv/ext/freertos/include.cmake
index e7ab7fd..70ed090 100644
--- a/lib/pbdrv/ext/freertos/include.cmake
+++ b/lib/pbdrv/ext/freertos/include.cmake
@@ -1,5 +1,5 @@
 target_sources(pbdrv PRIVATE "${CMAKE_CURRENT_LIST_DIR}/pb-mem.c")
-target_link_libraries(pbdrv
+target_link_libraries(pbdrv PUBLIC
 	freertos_kernel
 	freertos_kernel_include
 	freertos_config
diff --git a/lib/pbdrv/index.dox b/lib/pbdrv/index.dox
index eb0fd63..8ddcb6a 100644
--- a/lib/pbdrv/index.dox
+++ b/lib/pbdrv/index.dox
@@ -11,8 +11,9 @@ themselves. For a complete puzzle module driver, please see \ref pbdrv-mod.
 
 If you order to use \ref pbdrv, you need to include this folder in your
 CMakeLists.txt file, include the \ref pb_ext "extension" for your target
-platform, and link the \c pbdrv library with your executable:
+platform, and link the \c pbdrv library with your executable.
 
+\par Example
 ```cmake
 # include pbdrv
 add_subdirectory(lib/pbdrv)
@@ -35,14 +36,10 @@ using an existing \ref pb_drv "driver", or (partially) implementing the driver
 functions.
 
 Like \ref pbdrv, \ref pbdrv-mod can be used by including this folder in your
-CMakeLists.txt file and linking the library with your executable. A notable
-difference with \ref pbdrv-mod is that you do not need to include an extension.
-\ref pb_ext "Extensions" are still used by \ref pbdrv-mod, but they are
-included automatically by the target platform's \ref pb_drv "driver". The
-appropriate \ref pb_drv "driver" to load is also automatically detected.
-
-Example:
+CMakeLists.txt file and linking the library with your executable. Please see
+the \ref pb_drv "drivers" page for target-specific usage instructions.
 
+\par Example
 ```cmake
 # include pbdrv
 add_subdirectory(lib/pbdrv)
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/i2c.c b/main/i2c.c
index 0f23a13..db859d8 100644
--- a/main/i2c.c
+++ b/main/i2c.c
@@ -8,7 +8,6 @@
 
 #include "i2c.h"
 #include "pb-mod.h"
-#include "pbdrv.h"
 #include "config.h"
 #include "pb-buf.h"
 #include "pb-send.h"
diff --git a/main/init.c b/main/init.c
index d85c94d..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();
diff --git a/main/mod.c b/main/mod.c
index 11a1bb7..ac9107d 100644
--- a/main/mod.c
+++ b/main/mod.c
@@ -1,6 +1,13 @@
+#include <FreeRTOS.h>
+#include <task.h>
+
 #include "pb.h"
 #include "pb-mod.h"
 
 const char * PB_MOD_NAME = "main controller";
 const i2c_addr_t PB_MOD_ADDR = PB_ADDR_MOD_MAIN;
 
+void pb_mod_blocking_delay_ms(unsigned long ms) {
+	vTaskDelay(ms / portTICK_PERIOD_MS);
+}
+
diff --git a/main/pbdrv.c b/main/pbdrv.c
deleted file mode 100644
index 6a89253..0000000
--- a/main/pbdrv.c
+++ /dev/null
@@ -1,81 +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_S i2c0
-#define PB_I2C_M i2c1
-
-#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) {
-	return pb_i2c_recv(msg->data, msg->size);
-
-	// 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_S);
-			break;
-		}
-		case I2C_SLAVE_FINISH: {
-			msg_complete(msg);
-			break;
-		}
-		default: break;
-	}
-}
-
-void pb_setup() {
-	i2c_init(PB_I2C_S, PB_CLOCK_SPEED_HZ);
-	i2c_init(PB_I2C_M, PB_CLOCK_SPEED_HZ);
-
-	i2c_slave_init(PB_I2C_S, PB_MOD_ADDR, &recv_event);
-}
-
-__weak void pb_i2c_send(i2c_addr_t addr, const uint8_t * buf, size_t sz) {
-	// false to write stop condition to i2c bus
-	i2c_write_timeout_us(PB_I2C_M, addr, buf, sz, false, PB_TIMEOUT_US);
-}
-
-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 89a4870..0000000
--- a/main/pbdrv.h
+++ /dev/null
@@ -1,49 +0,0 @@
-#pragma once
-
-#include "pb-types.h"
-
-/**
- * \ingroup pb_drv
- * \defgroup pb_drv_rp2040 RP2040
- * \brief Raspberry Pi Pico and Pico W driver
- *
- * \note This file is no longer inside `lib/pbdrv/drv/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.
- *
- * \warning The workaround implemented in this driver was already kind of
- * messy, and **a different microcontroller should be used for the main
- * controller instead**. Please see the handover document for more details.
- *
- * \{
- */
-
-#ifdef __cplusplus
-extern "C" {
-#endif
-
-//! Puzzle bus driver setup
-void pb_setup();
-
-/**
- * \note 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.
- *
- * \warning 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);
-
-/// \}
-
-#ifdef __cplusplus
-}
-#endif
-
diff --git a/puzzle/dummy/CMakeLists.txt b/puzzle/dummy/CMakeLists.txt
index c2633a1..7edce2b 100644
--- a/puzzle/dummy/CMakeLists.txt
+++ b/puzzle/dummy/CMakeLists.txt
@@ -14,7 +14,7 @@ set(CMAKE_TOOLCHAIN_FILE ${CMAKE_SOURCE_DIR}/lib/Arduino-CMake-Toolchain/Arduino
 set(ARDUINO_BOARD "Arduino Uno [avr.uno]")
 # set(ARDUINO_BOARD "Arduino Mega or Mega 2560 [avr.mega]")
 
-# freertos
+# freertos (used for memory management only)
 add_library(freertos_config INTERFACE)
 target_include_directories(freertos_config SYSTEM INTERFACE .)
 set(FREERTOS_PORT GCC_ATMEGA)
@@ -28,6 +28,7 @@ set(FREERTOS_HEAP 4)
 project(pb_mod_dummy C CXX)
 
 add_subdirectory(lib/pbdrv)
+include(lib/pbdrv/ext/freertos/include.cmake)
 add_subdirectory(lib/FreeRTOS-Kernel)
 
 add_executable(main
-- 
cgit v1.2.3


From 46eb2370e53c3a1c7eb593ca99c07944c81ec695 Mon Sep 17 00:00:00 2001
From: Loek Le Blansch <loek@pipeframe.xyz>
Date: Sun, 23 Jun 2024 13:37:17 +0200
Subject: remove delay function from pbdrv

---
 lib/pbdrv/ext/freertos/include.cmake |  2 --
 lib/pbdrv/ext/freertos/index.dox     |  2 +-
 lib/pbdrv/ext/freertos/pb-mod.c      | 10 ----------
 lib/pbdrv/pb-mod.h                   |  8 --------
 main/i2c.c                           |  2 +-
 main/mod.c                           |  4 ----
 6 files changed, 2 insertions(+), 26 deletions(-)
 delete mode 100644 lib/pbdrv/ext/freertos/pb-mod.c

(limited to 'main')

diff --git a/lib/pbdrv/ext/freertos/include.cmake b/lib/pbdrv/ext/freertos/include.cmake
index 70ed090..e205c8f 100644
--- a/lib/pbdrv/ext/freertos/include.cmake
+++ b/lib/pbdrv/ext/freertos/include.cmake
@@ -5,5 +5,3 @@ target_link_libraries(pbdrv PUBLIC
 	freertos_config
 	)
 
-target_sources(pbdrv-mod PRIVATE "${CMAKE_CURRENT_LIST_DIR}/pb-mod.c")
-
diff --git a/lib/pbdrv/ext/freertos/index.dox b/lib/pbdrv/ext/freertos/index.dox
index dfa45ff..7d72918 100644
--- a/lib/pbdrv/ext/freertos/index.dox
+++ b/lib/pbdrv/ext/freertos/index.dox
@@ -2,5 +2,5 @@
 /**
 \ingroup pb_ext
 \defgroup pb_ext_freertos FreeRTOS
-\brief FreeRTOS memory management and scheduler-based delay
+\brief FreeRTOS memory management
 */
diff --git a/lib/pbdrv/ext/freertos/pb-mod.c b/lib/pbdrv/ext/freertos/pb-mod.c
deleted file mode 100644
index 5c0aa36..0000000
--- a/lib/pbdrv/ext/freertos/pb-mod.c
+++ /dev/null
@@ -1,10 +0,0 @@
-#include <FreeRTOS.h>
-#include <task.h>
-
-#include "../../pb-types.h"
-
-/// \ingroup pb_ext_freertos
-__weak void pb_mod_blocking_delay_ms(unsigned long ms) {
-	vTaskDelay(ms / portTICK_PERIOD_MS);
-}
-
diff --git a/lib/pbdrv/pb-mod.h b/lib/pbdrv/pb-mod.h
index 3869e55..91e1d1f 100644
--- a/lib/pbdrv/pb-mod.h
+++ b/lib/pbdrv/pb-mod.h
@@ -24,14 +24,6 @@ extern const char * PB_MOD_NAME;
  */
 extern const i2c_addr_t PB_MOD_ADDR;
 
-/**
- * \brief Platform-specific blocking delay function
- *
- * FIXME: this entire function should be removed (see handover: RP2040 I2C
- * limitations)
- */
-void pb_mod_blocking_delay_ms(unsigned long ms);
-
 /// \}
 
 /**
diff --git a/main/i2c.c b/main/i2c.c
index db859d8..93b64db 100644
--- a/main/i2c.c
+++ b/main/i2c.c
@@ -44,7 +44,7 @@ void bus_task() {
 		state_exchange();
 
 		// wait 1 second
-		pb_mod_blocking_delay_ms(1e3);
+		vTaskDelay(1e3 / portTICK_PERIOD_MS);
 	}
 }
 
diff --git a/main/mod.c b/main/mod.c
index ac9107d..8650861 100644
--- a/main/mod.c
+++ b/main/mod.c
@@ -7,7 +7,3 @@
 const char * PB_MOD_NAME = "main controller";
 const i2c_addr_t PB_MOD_ADDR = PB_ADDR_MOD_MAIN;
 
-void pb_mod_blocking_delay_ms(unsigned long ms) {
-	vTaskDelay(ms / portTICK_PERIOD_MS);
-}
-
-- 
cgit v1.2.3


From bfa85b6c313a725afe98bbe6ccb60978de3187a9 Mon Sep 17 00:00:00 2001
From: Loek Le Blansch <loek@pipeframe.xyz>
Date: Sun, 23 Jun 2024 13:54:02 +0200
Subject: add cross-reference for overwritten pbdrv handlers in main controller
 software

---
 lib/pbdrv/pb-route.h |  4 +++-
 main/i2c.c           | 11 +++++++++++
 main/i2c.h           |  1 +
 main/index.dox       |  7 +++++++
 4 files changed, 22 insertions(+), 1 deletion(-)

(limited to 'main')

diff --git a/lib/pbdrv/pb-route.h b/lib/pbdrv/pb-route.h
index b80e4ec..2a28c0b 100644
--- a/lib/pbdrv/pb-route.h
+++ b/lib/pbdrv/pb-route.h
@@ -98,6 +98,7 @@ void pb_route_cmd_state_req(pb_msg_t * msg);
  * controller handles this type of command.
  */
 void pb_route_cmd_state_res(pb_msg_t * msg);
+// TODO: add link to pb_route_cmd_state_res handler in main/i2c.c
 /**
  * \brief Handle a \ref PB_CMD_STATE "STATE" message with action \ref
  * PB_ACTION_SET "SET"
@@ -122,7 +123,8 @@ void pb_route_cmd_magic_req(pb_msg_t * msg);
  * PB_ACTION_RES "RES"
  *
  * The default implementation of this function is empty, as only the main
- * controller handles this type of command.
+ * controller handles this type of command. (\ref main_route_cmd_magic_res
+ * "link")
  */
 void pb_route_cmd_magic_res(pb_msg_t * msg);
 
diff --git a/main/i2c.c b/main/i2c.c
index 2503560..f366793 100644
--- a/main/i2c.c
+++ b/main/i2c.c
@@ -41,6 +41,17 @@ void bus_task() {
 	}
 }
 
+/**
+ * \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;
 	modules[modules_size++] = msg->sender;
diff --git a/main/i2c.h b/main/i2c.h
index 107a04d..27c0b02 100644
--- a/main/i2c.h
+++ b/main/i2c.h
@@ -15,3 +15,4 @@
 void bus_task();
 
 /// \}
+
diff --git a/main/index.dox b/main/index.dox
index aa2d07a..0fee58a 100644
--- a/main/index.dox
+++ b/main/index.dox
@@ -4,3 +4,10 @@
 \defgroup main_tasks tasks
 \brief Tasks
 */
+
+/**
+\ingroup main
+\defgroup main_pb_override overrides
+\brief Override functions from \ref pbdrv-mod
+*/
+
-- 
cgit v1.2.3


From 2528775b34f9c0033027931385a4de866ab2749d Mon Sep 17 00:00:00 2001
From: Loek Le Blansch <loek@pipeframe.xyz>
Date: Mon, 24 Jun 2024 08:31:35 +0200
Subject: add more documentation

---
 client/cmd.h      |  7 ++++++-
 client/i2c.h      |  2 +-
 client/parse.h    |  4 ++--
 client/sock.h     |  2 +-
 main/config.def.h | 27 ++++++++++++++++++++++++++-
 main/index.dox    |  6 +++---
 main/readme.md    | 12 +-----------
 7 files changed, 40 insertions(+), 20 deletions(-)

(limited to 'main')

diff --git a/client/cmd.h b/client/cmd.h
index a3cc44a..4f77d50 100644
--- a/client/cmd.h
+++ b/client/cmd.h
@@ -2,8 +2,13 @@
 
 /**
  * \ingroup pbc
- * \defgroup pbc_cmd cmd
+ * \defgroup pbc_cmd Commands
  * \brief Commands within \ref pbc
+ *
+ * \note A manpage is available containing end-user usage instructions inside
+ * the \ref client folder in the source code repository. This page contains the
+ * internal code documentation for the commands defined in \c pbc.
+ *
  * \{
  */
 
diff --git a/client/i2c.h b/client/i2c.h
index 87f33c9..d2cfa9a 100644
--- a/client/i2c.h
+++ b/client/i2c.h
@@ -5,7 +5,7 @@
 
 /**
  * \ingroup pbc
- * \defgroup pbc_i2c i2c
+ * \defgroup pbc_i2c I2C
  * \brief I2C abstraction functions
  * \{
  */
diff --git a/client/parse.h b/client/parse.h
index 8b7d235..1beb714 100644
--- a/client/parse.h
+++ b/client/parse.h
@@ -4,8 +4,8 @@
 
 /**
  * \ingroup pbc
- * \defgroup pbc_parse parse
- * \brief Debug send command parser utilities
+ * \defgroup pbc_parse Parse
+ * \brief Debug \c send command parser utilities
  * \{
  */
 
diff --git a/client/sock.h b/client/sock.h
index c124e45..792123e 100644
--- a/client/sock.h
+++ b/client/sock.h
@@ -5,7 +5,7 @@
 
 /**
  * \ingroup pbc
- * \defgroup pbc_sock sock
+ * \defgroup pbc_sock Socket
  * \brief TCP socket handling
  * \{
  */
diff --git a/main/config.def.h b/main/config.def.h
index b3be5ed..cb6e8b4 100644
--- a/main/config.def.h
+++ b/main/config.def.h
@@ -4,8 +4,33 @@
 
 /**
  * \ingroup main
- * \defgroup main_config config
+ * \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.
+ *
  * \{
  */
 
diff --git a/main/index.dox b/main/index.dox
index 0fee58a..54c5b15 100644
--- a/main/index.dox
+++ b/main/index.dox
@@ -1,13 +1,13 @@
 // vim:ft=doxygen
 /**
 \ingroup main
-\defgroup main_tasks tasks
-\brief Tasks
+\defgroup main_tasks Tasks
+\brief FreeRTOS tasks
 */
 
 /**
 \ingroup main
-\defgroup main_pb_override overrides
+\defgroup main_pb_override Overrides
 \brief Override functions from \ref pbdrv-mod
 */
 
diff --git a/main/readme.md b/main/readme.md
index 85a3fca..97150eb 100644
--- a/main/readme.md
+++ b/main/readme.md
@@ -8,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
-- 
cgit v1.2.3


From 3e7765dd54706eb927b653d1ed8fa03a1f288c88 Mon Sep 17 00:00:00 2001
From: Loek Le Blansch <loek@pipeframe.xyz>
Date: Mon, 24 Jun 2024 09:45:15 +0200
Subject: ignore some addresses in bus scan

---
 main/i2c.c | 6 ++++++
 1 file changed, 6 insertions(+)

(limited to 'main')

diff --git a/main/i2c.c b/main/i2c.c
index 87c8161..2237b4d 100644
--- a/main/i2c.c
+++ b/main/i2c.c
@@ -12,6 +12,9 @@
 #include "pb-buf.h"
 #include "pb-send.h"
 
+// stolen from lib/pico-sdk/src/rp2_common/hardware_i2c/i2c.c
+#define i2c_reserved_addr(addr) (((addr) & 0x78) == 0 || ((addr) & 0x78) == 0x78)
+
 i2c_addr_t modules[CFG_PB_MOD_MAX];
 size_t modules_size = 0;
 
@@ -21,6 +24,9 @@ static void bus_scan() {
 	// 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);
 	}
 
-- 
cgit v1.2.3