TcMenu / IoAbstraction

Rotary encoders, fully debounced switches, EEPROM support on Arduino and mbed - direct and over I2C
Apache License 2.0
162 stars 33 forks source link

Does not seem to compile with RP2040 and Arduino-mbed #147

Closed dogtopus closed 2 years ago

dogtopus commented 2 years ago

I tried to setup a development environment with PlatformIO and Raspberry Pi Pico, which uses Arduino-mbed, and the compilation printed a lot of errors starting with something like

In file included from .pio/libdeps/pico/TaskManagerIO/src/TaskManagerIO.cpp:6:
.pio/libdeps/pico/TaskManagerIO/src/TaskPlatformDeps.h: In function 'void* getCurrentThreadId()':
.pio/libdeps/pico/TaskManagerIO/src/TaskPlatformDeps.h:30:44: error: 'ThisThread' has not been declared
   30 | inline void* getCurrentThreadId() { return ThisThread::get_id(); }
      |                                            ^~~~~~~~~~

What my platformio.ini looks like:

[env:pico]
platform = raspberrypi
board = pico
framework = arduino

lib_deps =
    SPI
    Wire
    davetcc/tcMenu
    davetcc/IoAbstraction
    davetcc/TaskManagerIO
dogtopus commented 2 years ago

Now look at it more closely, it seems to be a TaskManagerIO issue. Probably should transfer it there.

dogtopus commented 2 years ago

Solved. Have to force TaskManagerIO into Arduino-mbed mode by specifying -DARDUINO_ARDUINO_NANO33BLE (and -DPIO_NEEDS_RTOS_WORKAROUND because davetcc/TaskManagerIO#17).

Maybe we should also whitelist RP2040 Arduino as Arduino-mbed? Looks like Arduino Nano RP2040 Connect is also using it. A better longterm solution would be detecting Arduino-mbed directly so no more whitelisting is necessary.

dogtopus commented 2 years ago

See #148 and davetcc/TaskManagerIO#38

davetcc commented 2 years ago

OK great thanks, that is now merged, I'll leave this open to check if any subsequent changes are needed either here or in SimpleCollections.

davetcc commented 2 years ago

I think that this will need the same change to be made:

https://github.com/davetcc/IoAbstraction/blob/ebf8680a25054c1e914213c25eedcce3b5ceaa28/src/PlatformDetermination.h#L17

Possibly also in SimpleCollections for the threading support. I'm doing some work on that at the moment..

dogtopus commented 2 years ago

I think that this will need the same change to be made:

https://github.com/davetcc/IoAbstraction/blob/ebf8680a25054c1e914213c25eedcce3b5ceaa28/src/PlatformDetermination.h#L17

This should be implemented in #148.

Possibly also in SimpleCollections for the threading support. I'm doing some work on that at the moment..

From a quick search, yes. It's necessary for proper CAS locking support but it might not fail immediately.

davetcc commented 2 years ago

Thanks a lot for the PR. I've just merged it.

For simple collections the case is easier, it just needs to know if it is mbed based, so that one is very easy, just check for MBED and remove anything else. In fact longer term, I could move all mbed choices to use the LDREX/STREX hardware CAS code I've written, it is implicitly safe in all scenarios on any ARM cortex 4 and above. But I was trying to introduce that code slowly.

davetcc commented 2 years ago

Plan of action:

davetcc commented 2 years ago

A better longterm solution would be detecting Arduino-mbed directly so no more whitelisting is necessary.

Agreed, it's not optimal, looking at the build flags in platformIO, I wonder if DARDUINO_ARCH_MBED would work longer term doesn't defined only by pio.. The only one even remotely possible looks like TARGET_LIKE_MBED but is that only defined by Arduino - IE not if someone is using mbed It is set by mbed as well. There is no easy way to determine apart from on PlatformIO :(

I would say, release what we have for now. I'll try and take a look through the build paramters for all the boards in Arduino and platformIO. As long as that is always defined, it would be safe to use.

davetcc commented 2 years ago

I've taken all the changes locally, and simplified the SimpleCollections check down to just __MBED__, added another small IoAbstraction fix, and then tested.

So far testing looks good on:

Left to do:

I've got another very small change in IoAbstraction that I'm testing at the same time, once I finish the testing I'll do a release.