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

Update support/example for ESP32-C3 to use latest versions of dependencies #975

Merged
merged 13 commits into from
Oct 16, 2024

Conversation

jessebraham
Copy link
Contributor

@jessebraham jessebraham commented Sep 5, 2024

This updates the rtic and rtic-macros packages to handle some changes which have been made since support for the ESP32-C3 has been updated last. It additionally updates the sw_and_hw example for this device, which I have tested and verified using a ESP32-C3-DevKit-RUST-1 v1.2a board.

If there are any questions or concerns please let me know, I'm happy to make any requested changes.

This supersedes #973 as well, so that can be closed when/if this is merged.

}

// do nothing in init
#[init]
fn init(_: init::Context) -> (Shared, Local) {
println!("init");
let peripherals = Peripherals::take();
let io = IO::new(peripherals.GPIO, peripherals.IO_MUX);
let mut button = io.pins.gpio9.into_pull_down_input();
let io = Io::new_no_bind_interrupt(peripherals.GPIO, peripherals.IO_MUX);
Copy link
Contributor Author

@jessebraham jessebraham Sep 5, 2024

Choose a reason for hiding this comment

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

I will note that this is a bit of a rough edge for which we don't have a good solution yet. Essentially the issue is that Io::new() binds a handler to the GPIO interrupt for async functionality, so if we want to use this interrupt for other purposes (like in the sw_and_hw example, when creating the hardware task) then we need to not bind the interrupt in the constructor.

This is potentially a bit of a footgun, so I can add a comment here if necessary.

Copy link
Contributor Author

@jessebraham jessebraham Sep 6, 2024

Choose a reason for hiding this comment

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

There is a tracking issue for this in the esp-hal repo, FYI: esp-rs/esp-hal#2009

@jessebraham
Copy link
Contributor Author

I'm not sure what you'd like me to do about the QEMU failure, I would not expect this output to stay consistent (as we're seeing with this failure).

@jessebraham jessebraham mentioned this pull request Sep 5, 2024
4 tasks
@onsdagens
Copy link
Contributor

onsdagens commented Sep 6, 2024

Tested this locally on the same board, and it seems to work as expected.

I've fixed up the QEMU expected output, applied my suggestions and opened a PR on your fork.

@onsdagens
Copy link
Contributor

onsdagens commented Sep 6, 2024

i see my review is set as pending, so i'm not sure if you can see my comments. in any case, i see now what you were saying about QEMU, the output does not match what is expected, and what actually happens when running on a board

i think we should keep QEMU around, adjust the expected output to the unexpected results and use it as a marker for when something is potentially wrong and needs to be run on a board.

@jessebraham
Copy link
Contributor Author

I agree, it's probably worth keeping the checks around, however perhaps being a bit less strict with regards to the bootloader output would be best.

Anyway, with your changes merged CI is green here now at least, so thanks for that!

rtic/src/export/riscv_esp32c3.rs Outdated Show resolved Hide resolved
rtic/src/export/riscv_esp32c3.rs Outdated Show resolved Hide resolved
@onsdagens
Copy link
Contributor

i guess i just forgot to press submit, the comments are now outdated but i will leave them here for the record.

@korken89
Copy link
Collaborator

korken89 commented Sep 29, 2024

Great work, thank you!

I just merged some changes from @onsdagens , could you double check this PR is still OK?
E.g. there was a reference to esp32c3 v0.22 in rtic-montonics now as well.

@jessebraham
Copy link
Contributor Author

Thanks for taking a look. I have rebased my branch locally but am running into some issues with the monotonic example; will investigate further.

Might take me a few days to get to this, lots going on right now for me :)

@korken89
Copy link
Collaborator

Thank you, no rush!

@jessebraham jessebraham force-pushed the fixes/update-esp32c3-example branch from 42ea00c to 3e7998c Compare October 2, 2024 13:04
@jessebraham
Copy link
Contributor Author

jessebraham commented Oct 2, 2024

I've resolved the issues I had encountered with the monotonic example, seems to be working correctly now.

Unfortunately, again we are having issues with the checks on the QEMU output. Happy to make whatever changes here, but I am not very familiar with these checks so am not sure how best to resolve this long-term. I think that the bootloader output can probably be ignored entirely in the QEMU checks.

@AfoHT
Copy link
Contributor

AfoHT commented Oct 2, 2024

I agree z

I've resolved the issues I had encountered with the monotonic example, seems to be working correctly now.

Awesome! 🚀

Unfortunately, again we are having issues with the checks on the QEMU output. Happy to make whatever changes here, but I am not very familiar with these checks so am not sure how best to resolve this long-term. I think that the bootloader output can probably be ignored entirely in the QEMU checks.

I agree, bootloader is not the main part. Do you want to attempt implement the ignoring of bootloader output, or should we join forces on that one?

@jessebraham
Copy link
Contributor Author

Do you want to attempt implement the ignoring of bootloader output, or should we join forces on that one?

Would be great if I could get a hand with that, please. New project at work will be consuming a fair bit of my brainpower and time for at least the next couple weeks here.

@AfoHT AfoHT force-pushed the fixes/update-esp32c3-example branch from 0a8eeab to f6fd6ba Compare October 16, 2024 19:21
@AfoHT AfoHT enabled auto-merge October 16, 2024 19:21
Copy link
Contributor

@AfoHT AfoHT left a comment

Choose a reason for hiding this comment

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

🚀

@AfoHT AfoHT added this pull request to the merge queue Oct 16, 2024
Merged via the queue into rtic-rs:master with commit 1f6b6a4 Oct 16, 2024
55 checks passed
@jessebraham
Copy link
Contributor Author

Thank you for wrapping this up!

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