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

Add an example with RTIC. #27

Closed
wants to merge 2 commits into from
Closed

Add an example with RTIC. #27

wants to merge 2 commits into from

Conversation

matoushybl
Copy link

This PR adapts the ip example to use RTIC. I have tested the code on a F767, but adapted it to the F4 as the original example uses. That means that it couldn't be tested as I do not have a Nucleo with an F4 at hand.

Copy link
Member

@thalesfragoso thalesfragoso left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, I left a few nitpicks.

TBH, I think it would be good to have an example using a F7, so if it isn't a lot of work for you, it would be nice to change this to your original work, otherwise I'm fine with this too.

Comment on lines 67 to 68
static mut RX_RING: Option<[RingEntry<RxDescriptor>; 8]> = None;
static mut TX_RING: Option<[RingEntry<TxDescriptor>; 2]> = None;
Copy link
Member

Choose a reason for hiding this comment

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

Could you use the const constructors introduced in #22 ?

Copy link
Author

Choose a reason for hiding this comment

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

I used them, but the problem is that they are not copy so they cannot be used like before. They could be generated by a macro call. Is this solution good enough? Or do you propose something else?

Copy link
Member

Choose a reason for hiding this comment

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

Unfortunately, writing them out like that is required for now, at least until rust-lang/rust#49147 is solved. However, I think we export a type alias to help with that, RxRingEntry and TxRingEntry IIRC.

I think it's important to show this const option, without it you end up paying double the ram during initialization, and since those structs are quite large, it becomes required in some cases to avoid stack overflows.

// HCLK must be at least 25MHz to use the ethernet peripheral
let clocks = rcc.cfgr.sysclk(32.mhz()).hclk(32.mhz()).freeze();

writeln!(stdout, "Enabling ethernet...").unwrap();
Copy link
Member

Choose a reason for hiding this comment

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

Why don't use log instead of writeln!(stdout) ? That way you save some code and a RTIC resource.

Copy link
Author

Choose a reason for hiding this comment

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

I used the same thing used in the non-RTIC example for consistency, my code originally used defmt. What exactly do you mean by log? is there a Rust log backend for RTT?

Copy link
Member

Choose a reason for hiding this comment

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

You already implemented log for semihosting, can't you just call log::info! (etc) instead of writeln! ?

Copy link
Member

Choose a reason for hiding this comment

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

Oh, and about the defmt commented out code, maybe you could change it for log::error! ?

@maximeborges
Copy link
Member

Do you have an actually working example running on the F767 ? I'm not sure were in my setup things are broken, but I can't sniff a single thing with Wireshark with either your PR or any other example in the project that I tried to adapt to the F767.

@matoushybl
Copy link
Author

Yeah, I did. I was using the Nucleo board though.

@matoushybl
Copy link
Author

Have a look here: https://github.com/matoushybl/nucleo-playground

@maximeborges
Copy link
Member

Thanks!

@datdenkikniet
Copy link
Contributor

datdenkikniet commented Aug 9, 2022

We've added an up-to-date and working RTIC + smoltcp example in #54 that compiles and runs on all supported MCUs.

I'm fairly certain the example should run on nucleo boards with the rtic-echo-example-altpin feature. If not, please let me know and we'll figure out the pinout to get it to work.

Closing in favor of #54

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