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

examples/NonArduino/Tock: Support RISC-V and bump libtock-c #1082

Merged
merged 2 commits into from
May 1, 2024

Conversation

alistair23
Copy link
Contributor

No description provided.

@alistair23 alistair23 force-pushed the alistair/riscv-support branch 4 times, most recently from e2a13e2 to db1f9ba Compare May 1, 2024 09:44
Signed-off-by: Alistair Francis <alistair@alistair23.me>
@alistair23 alistair23 force-pushed the alistair/riscv-support branch 4 times, most recently from 4ca1c93 to 940486e Compare May 1, 2024 10:15
@jgromes
Copy link
Owner

jgromes commented May 1, 2024

I took a look in the CI runs and it looks like it's missing pretty basic stuff, like stdio and stdint:

https://github.com/jgromes/RadioLib/actions/runs/8907994652/job/24462852153#step:5:8662

To me that makes it look like the environment is not set up correctly. So I'm not sure this should be merged before that gets resovled. Disabling RISC-V build in CI is a rather strange choice considering that this PR adds RISC-V support.

@alistair23
Copy link
Contributor Author

What is happening is that the RadioLib cmake build environment in the CI isn't setup correctly for RISC-V, hence the missing stdio and stdint.

I can't reproduce the failures locally in Arch, as Arch has better RISC-V packages.

libtock-c also builds fine as it builds it's own standard libraries.

The problem is that I can't easily get the Make information into CMake to get it to build successfully in the CI. I can use RadioLib as a library with libtock-c (with a not yet submitted patch) to build a RISC-V application and I can build this PR successfully locally for RISC-V. The CI happily builds libtock-c for RISC-V as well. The stdio and stdint headers are provided as well as the libraries, so there is just something slightly wrong in the CMake setup that fails in the CI, but I'm a little stuck reproducing it.

@jgromes
Copy link
Owner

jgromes commented May 1, 2024

What is happening is that the RadioLib cmake build environment in the CI isn't setup correctly for RISC-V, hence the missing stdio and stdint.

But you have control over that environment, the tock-build job does nothing else than build tock. So you should be able to set it up in such a way that it can compile both ARM and RISC-V, otherwise it's a bit of a "it works on my machine" kind of situation ... ;)

@alistair23 alistair23 force-pushed the alistair/riscv-support branch 4 times, most recently from de5969a to adea485 Compare May 1, 2024 12:19
Signed-off-by: Alistair Francis <alistair@alistair23.me>
@alistair23
Copy link
Contributor Author

That's a fair point. I managed to fix it!

So now RISC-V is built in the CI. The script still provides an option to skip the RISC-V RadioLib build in case others have an issue.

Also the abs() function isn't officially include in math.h: https://www.techonthenet.com/c_language/standard_library_functions/stdlib_h/abs.php

@jgromes jgromes merged commit 2f85326 into jgromes:master May 1, 2024
30 checks passed
@jgromes
Copy link
Owner

jgromes commented May 1, 2024

Logging good, thank you very much!

Also the abs() function isn't officially include in math.h:

Thanks, RadioLib should be using fabs exclusively. Though there do seem to be two of them I forgot in FSK4.cpp, I will fix that right away.

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.

2 participants