place rp2040 back in pbdrv

18 files changed, 157 insertions, 204 deletions

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