TcMenu / TaskManagerIO

A task, event, scheduling, and interrupt marshalling library for Arduino and mbed boards.
Apache License 2.0
122 stars 12 forks source link

[Workaround] Library uncompilable for mbed 5 platformIO because of yield() function #17

Open maxgerhardt opened 3 years ago

maxgerhardt commented 3 years ago

Your code reads

https://github.com/davetcc/TaskManagerIO/blob/2840e53c91f4184497180c27c8bbafc851fdc10c/src/TaskManagerIO.cpp#L410-L412

However, this throws the error message

.pio\libdeps\disco_f051r8\TaskManagerIO\src\TaskManagerIO.cpp:404:5: error: 'ThisThread' has not been declared
  404 |     ThisThread::yield();
      |     ^~~~~~~~~~
*** [.pio\build\disco_f051r8\libca9\TaskManagerIO\TaskManagerIO.o] Error 1

and is fixed by explicitily including rtos/rtos.h and using rtos::ThisThread::yield();.

#include <rtos/rtos.h>
void yield() {
    //ThisThread::yield();
    rtos::ThisThread::yield();
}

The platformio.ini used here is

[env:disco_f051r8]
platform = ststm32
board = disco_f051r8
framework = mbed
lib_deps = 
    davetcc/LiquidCrystalIO @ ^1.4.0
    davetcc/IoAbstraction @ ^1.6.4

With the current versions of all platform that's using mbed-os 5.15.4. On the other hand, this compiles fine for the L476RG board which uses mbed 6.2.0. So I'm not entirely sure on what the correct fix here is, but primarily leaving this issue in case someone runs in the same compile error.

See reference project at https://github.com/maxgerhardt/pio-mbed-i2c-lcd-example/.

davetcc commented 3 years ago

Thanks for raising the issue. The changes between mbed 5 and mbed 6 have been a real pain point for me, when mbed 6 became the default I had to modify quite a bit of code to make it compile. However, I will look to see if that solution works with both versions, if it does, I'll use that by default. Otherwise I'll add an extra case into PlatformDetermination.h that detects the mbed version and works out which code to include.

davetcc commented 3 years ago

I'll try and get hold of an older mbed board that still compiles with mbed 5, as I think it will be generally useful - can you recommend a suitable part that still uses mbed 5? (I was looking at STM32L152C-DISCO STM32F0DISCOVERY).

I'm currently testing on a quite large mbed board, the stm32f439 unit, and it may be useful to have a smaller board to hand too.

davetcc commented 3 years ago

I see you've already marked on the project page that it doesn't work on the DiscoveryF0 board. I actually got one (not very expensive) and tried it, the memory is so tight that I couldn't even get Serial working, taskManager which is pretty light, and can run on an Uno comfortably cannot be loaded. I would recommend that you suggest a larger board on your page, there are many inexpensive mbed boards available that have sufficient resources to run mbed 5 & 6.

We only really envisaged using taskManager and IoAbstraction with mbed on moderate to large boards using RTOS. We thought most people wanting lighter would either go STM32Cube or Arduino. However, I've fixed the compile error and made a commit onto master, it just falls back to using the older wait method with a very small delay of 0.1us.

After a bit more checking, the discovery F0 boards are not marked as mbed compatible on the back of the box, and I suspect it's because memory is too tight to do anything useful. Given its a 32-bit device it will most likely have 4-byte wide instructions, so this is similar in flash size to an Uno, but trying to run a much larger platform! The smallest possible blink program uses 80% of the flash and statically allocates 50% of the Ram before it has started! There are less than 400 bytes left to allocate once the program starts, as the 350-byte allocation in task manager fails.

davetcc commented 3 years ago

So just to avoid doubt for anyone looking into this, the Discovery F051 is too small to run most mbed programs. It has so little RAM left after start-up that you cannot even allocate the small amount of memory needed for task manager (about 300 bytes).

We have made the task manager library compile on mbed 5, for backward compatibility. So if you have a large enough mbed 5 board, it should work, even without RTOS.

maxgerhardt commented 3 years ago

Yep indeed, the STM32F051R8 chip is too small for it, even with the fix applied, it overflows flash. I'll have a look through the PlatformIO boards to see which one is declared mbed-os 6 and which one's a 5.

What is weird about this though is the compilation fix. Usually when you do #include <mbed.h> it autodetects if you're running an RTOS and does #include <rtos/rtos.h> and using namespace rtos; so really between mbed 6 and 5 both ThisThread::yield(); should be working with a #include <mbed.h> I expect.

davetcc commented 3 years ago

Agreed, but I even had problems getting rtos::ThisThread::yield() working when I conditionally included it on mbed 5. It would compile but not link for me basically. Given there is no rtos in this case, maybe that is not surprising. Given that wait(...) was still available in mbed 5, I opted for that just for now. Reading the mbed 5 and 6 documentation, it seems that I should be able to use the ThisThread functions in all cases. Maybe there is something missing somewhere..

davetcc commented 3 years ago

I'll bring this up on the PlatformIO community forum, as I think this is a wider issue, maybe it will need one of us to compare behaviour against the web IDE or mbed CLI.

davetcc commented 3 years ago

I've put some analysis here: see https://community.platformio.org/t/mbed-rtos-baremetal-and-thisthread-compilation-issues/17555

davetcc commented 3 years ago

So after doing more research on this, it looks like there may be an issue somewhere with RTOS support that needs some deeper investigation (see the link above). For now, I'll introduce a build level define that will need to be set for platform IO when NOT using an RTOS capable board. I don't think anyone using mbed cli/IDE would need this.

build_flags = -DPIO_NEEDS_RTOS_WORKAROUND

This forces platform determination not to include rtos.h and to use wait(..) instead of ThisThread::yield(). I am fairly confident that this would not be needed when using a regular mbed build. This is a temporary flag until there is certainty around a long term solution.

davetcc commented 3 years ago

I'm going to do a release of this shortly, if you're happy with the temporary solution in the last comment, please close out the issue. Obviously not a long term solution, but I actually think that the problem does not lie in task manager.