[embassy-net-nrf91] patch greater rx_data_len size

[wieerwill]

Switching from a NRF9160 MCU to the newer NRF9151 there were some changes regarding LTE connections. I used the same code on both boards.

1. the NRF9151 switched it's logic to use the external oscillator by default; this can be changed by setting HFXOSRC to 0 and reboot. In the same step it's also recommended and verified to use hfclk_source = HfclkSource::Internal in your init config, e.g.:

   let mut peripheral_config = embassy_nrf::config::Config::default();
   peripheral_config.hfclk_source = HfclkSource::Internal;
   peripheral_config.lfclk_source = LfclkSource::InternalRC;
   let p = embassy_nrf::init(peripheral_config);
   

Except of course if you got an explicit external clock in use.

2. starting up the modem with correct configuration leads to a endless wait state while the modem starts up. Issue covered up here was a change in the rx_control_len from uint 16 to 32. Changing this leads to usage as usual.

Tested on the NRF9160-DK and a NRF9151 testboard.

Done:

• describe errors and solution
• add nrf9151 feature to crate; crate can be used as is in older projects and patched for newer MCU
• patch rx_data_length
• assert_eq got unique error messages
• add feature note to Readme

TODO: examples to test and add to examples folder (possibly other PR in future)

[Dirbaio]

The modem firmware tells us the list length for rx. It would be better to just use it, instead of validating it equals 16 or 32.

This is better because

• it doesn't require a Cargo feature
• It'll work if in the future Nordic decides to change the list length to e.g. 42.

It'll need a small refactor to not use a fixed-length array for the rx list. Could you do that?

[wieerwill]

@Dirbaio thanks for your fast reply.

I too first thought that in the code we could just remove the assert_eq but it comes with faults: a change or defect in the modem would go unnoticed; e.g. a faulty value, wrong read address or error in the unsafe access could cause greater errors later on. As the LIST_LEN is used at other parts of the code too, it could cause problems if a length is read that is not divide-able by 8/16/32.

With the feature the check is still in place. With the assert error messages any modem change at this part is easy fixable and searchable. Future Changes of any nRF MCU can be detected with that with ease. And if Nordic decides to change the list length again it can be added with a clear visible and separated feature too.

If you still suggest to just use the the given list length from the modem, add this comment a 🚀 and i refactor it to not use a fixed-length array.

[Dirbaio]

Yes, we really should change it.

There's two lists:

• For TX (ie application -> modem), we decide the length, we allocate it, we tell the modem the length and address. Code for this can use LIST_LEN and fixed-size arrays.
• For RX (ie modem -> application), the modem decides the length and allocates it. It tells us the length and address.

Nordic's lib uses the len provided by the modem. When I initially implemented embassy-net-nrf91, I saw the RX list length seemed to be always 16 and decided to just hardcode it because it was easier.

With modem firmware 2.0.0 (available for nrf9151, not for nrf9160) they changed the list length to 32.

[Dirbaio]

New version is looking good! :)

Seems something went wrong while merging/rebasing, there's now 300 commits in the PR. Could you fix that? I'd also suggest squashing it down to one commit so the previous approach doesn't show up in the git history.

[wieerwill]

cleaned and squashed the PR

with those changes i can access and communicate with both the nrf9160 and nrf9151 modem (verified trough AT commands). Sadly i only get the nrf9160 to establish a connection but not the nrf9151. the nrf9151 loops endless at stack.wait_config_up(), to make up for other faults i added the timeout as the on-board modem shuts itself down after 60 seconds.

currently i can not establish a LTE/NB-IoT connection with the nrf9151 and opened a question regarding this in the Nordic DevZone: https://devzone.nordicsemi.com/f/nordic-q-a/122021/nrf9151-replaces-nrf9160-lte-not-working-any-more

[wieerwill]

Hi @diondokter you were recommended to me and i saw you wrote the nrf-modem crate and did some embassy work here too. could you please take a look into this?

[diondokter]

Hi, I've never looked at the internals of libmodem properly, so I don't have a lot of info on that front. Not sure what I can help with...

[wieerwill]

Hi, I've never looked at the internals of libmodem properly, so I don't have a lot of info on that front. Not sure what I can help with...

thanks for your lightning fast reply. could you verify the changes of this PR work on a NRF9151? i got a hard time testing and figuring out the internals of the modem and NRF and only got a custom PCB instead of the DevBoard therefore i want to make sure this works and does not throw up elsewhere. especially connecting to LTE/NB-IoT Networks

[diondokter]

I only have an nRF9160 :( All the support for other 91-series chips in nrf-modem has been submitted by other people (but libmodem smoothes over them a lot)

[lulf]

fyi, I got hold of an nrf9151, so I will give it a spin in the coming days.

[Dirbaio]

Thanks for the PR! I've rebased it and removed unrelated things to the main point of the PR (nrf9151)

• Removed the attach timeout. In async Rust it's generally idiomatic to not build timeouts into things, because the user can add their own from outside with with_timeout or select.
• Removed URC printing. It's nice to have, feel free to send it again in a separate PR. IMO we should ideally add a proper public API for it insetad of just logging it.
• Kept only some of the extra tracing