maxgerhardt / pio-pico-core-earlephilhower-test

Test firmware that uses the earlephilhower Arduino core.
4 stars 2 forks source link

Tone library glitch #3

Closed jhmaloney closed 2 years ago

jhmaloney commented 2 years ago

Hi, Max.

I noticed a strange problem with the Tone library; it doesn't appear to be finding existing entries in _toneMap. I reported the symptoms and a test program here but it turns out that the problem is in your fork, not the release version of Earle's Pico framework.

After doing some debugging, the issue appears to be that the find() operation is failing to find the existing tone entry for that pin:

    auto entry = _toneMap.find(pin);
    Tone *newTone;
    if (entry == _toneMap.end()) { // this is always true, even when there was already an entry for the given pin

As a result, it creates a new tone instance and associated state machine.

The code looks correct, but I don't think it is behaving as intended. Unfortunately, I'm not enough of C++ expert to know why find() isn't returning the existing entry. Looks like it could be a bug in the std:map class.

Is there an easy way to pull the release version of Earle's framework into your platformio fork?

Thanks!

maxgerhardt commented 2 years ago

There may be a possible linking issue, the stdc++ library that surely contains the std::map::find() function is in libstdc++.a. I'll double-check whether all linking and compile commands are the same in PlatformIO as they are in the Arduino IDE.

I've also had a report of interrupts not correctly firing, which maybe related to this general issue.

maxgerhardt commented 2 years ago

I don't yet know why, but even creating a std::string crashes, the internal string allocation routines somehow trigger the isr_svcall (supervisor call) interrupt, which is just a function with a breakpoint.

grafik

I'll have a look at the linking settings..

maxgerhardt commented 2 years ago

FOUND the culprit. The Arduino-Pico core has a lib/ folder with libpico.a and libstdc++.a. It links in libpico.a using its full filepath as an input to the linker, and also does -lstdc++ to link in the standard C++ library. HOWEVER the linker will pick up its compiler-internal libstdc++.a, and not Arduino-Pico's lib/libstdc++.a file then, especially since no -L flag was given that adds the lib/ folder to the linker search path. PlatformIO adds the lib/ folder to the linker search path via -L and links in both -lpico and -lstdc++.

Renaming the file C:\Users\<user>\.platformio\packages\framework-arduinopico\lib\libstdc++.a into e.g. libnotstdc++.a makes my minimal std::map project work again.

I'm wondering why the stdc++ library is there as a file but not linked in.. Anyways, I have to fix the builder script now to only link in libpico with its full path without making use of a -L flag..

CC / FYI @earlephilhower

maxgerhardt commented 2 years ago

Fixed in https://github.com/maxgerhardt/arduino-pico/commit/06fb23595ffd0379ffcb96b66fd8df6a41aba7b9, will cherry-pick into Earle's core too.

@jhmaloney please update again with git pull && git submodule update in the C:\Users\<user>\.platformio\packages\framework-arduinopico folder, rebuild the firmware and check if it's working for you now too.

jhmaloney commented 2 years ago

Wow, that was a strange problem. Excellent detective work to track it down so quickly. Strange that the two libstc++.a files are not an issue when using the Arduino IDE. Maybe the get around that by using the -L flag or explicit paths.

I did git pull && git submodule update and rebuilt my firmware and it sounds as though it is working perfectly. I will check it with a 'scope tomorrow.

I see that there are actually four libstdc++.a libraries in the Pico toolchain:

arm-none-eabi/lib/thumb/autofp/v7/fpu/libstdc++.a arm-none-eabi/lib/thumb/libstdc++.a arm-none-eabi/lib/arm/autofp/v5te/fpu/libstdc++.a arm-none-eabi/lib/libstdc++.a

I guess those cover all the combinations of fpu and thumb instructions. I know the RP2040 doesn't have an FPU. Does it always use thumb instructions? Just curious...

Thanks a lot for the quick fix!