Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updated CMake for latest Pico-SDK, including Pico2 support (issue #18) #19

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

sean-lawless
Copy link

... also includes hello_world fix (issue #17). Upgraded C++ compiler to 2017. NOTE: Four (4) tests are now skipped due to missing C++ linker dependencies - if merged please open an issue for these.

@lurch
Copy link

lurch commented Oct 2, 2024

NOTE: Four (4) tests are now skipped due to missing C++ linker dependencies

As GitHub isn't doing a very good job of rendering the diff here, it might be worth mentioning on this PR which of the tests are now skipped?

@sean-lawless
Copy link
Author

sean-lawless commented Oct 2, 2024

NOTE: Four (4) tests are now skipped due to missing C++ linker dependencies

As GitHub isn't doing a very good job of rendering the diff here, it might be worth mentioning on this PR which of the tests are now skipped?

Apologies, it looks like I introduced a line ending issue with the CMakeLists.txt (thank you Windows)...

The commented tests are:
#add_subdirectory("tests/flatbuffer_utils_test")
#add_subdirectory("tests/kernels_elementwise_test")
#add_subdirectory("tests/kernels_l2_pool_2d_test")
#add_subdirectory("tests/kernels_l2norm_test")

The errors are link related, For example:

[flatbuffer_utils_test]: undefined reference to `std::__throw_length_error(char const*)
...
[kernels_l2norm_test]: undefined reference to `sqrtf'

@sean-lawless
Copy link
Author

I fixed the line endings so CMakeLists.txt shows changes.

@petewarden
Copy link
Collaborator

Apologies for the delay on this one, I was missing the notifications (fixed now).

I actually have a prerelease 2350 and was able to get TFLM compiling with minor changes. I will return to that now the SDK is final.

One wrinkle is that this repo is actually generated from the upstream Google TFLM repo and then patched with a script. That means if we want to continue to be able to merge upstream changes, we'll need to make sure the RP2xxx changes are part of the patch process, not direct modifications.

Anyway, sorry again for the lack of love, I should have time this week to focus on this update.

@Hedda
Copy link

Hedda commented Dec 24, 2024

@sean-lawless FYI, pete has now committed update from latest upstream SDK -> 82ee61d

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, looks like this is a fix for a separate bug from the Pico2 support? I'll dig into this and see where the incorrect code is coming from. If it's upstream, I'll add a patch in the sync process, otherwise I'll pull this part of the PR in directly. Thanks for the update!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants