From 648d87ea98ec39d5745d36a0b5c5078cd9491211 Mon Sep 17 00:00:00 2001 From: Loek Le Blansch 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 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 #include -#include -#include -#include - #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 ) 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 +#include +#include + +#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 +#include + #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 -#include -#include -#include - -#include -#include -#include - -#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