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

InvalidAddr warning reported and non-working advertisements with empty scan_data and recent rustc #283

Open
peter9477 opened this issue Dec 3, 2024 · 2 comments

Comments

@peter9477
Copy link

We'd been pinned to rust v1.75 (nightly 2023-08-08) with code that worked fine when passing in scan_data: &[] when it built a ConnectableAdvertisement::ScannableUndirected advertisement. We used only the adv_data portion, and didn't want any scan data.

Once we tried going to rust versions starting around 2024, including with the recent rust v1.85 (nightly-2024-12-01), this code still compiled but led to the advertise_connectable() call logging a warning, and no advertisements showing up.

220.121 WARN : nrf_softdevice::ble::peripheral || sd_ble_gap_adv_set_configure err InvalidAddr
220.121 WARN : nrf_softdevice::ble::peripheral || sd_ble_gap_adv_stop: InvalidState

After experimentation we found a viable workaround, apparently without negative side-effects, by passing in scan_data: &[0u8] instead.

I don't understand the cause yet. It appears consistent with the SoftDevice applying an unnecessary check that any non-NULL pointer must have non-zero length data. The nrf-softdevice code provides no way to ensure that an actual NULL pointer is passed into the sd_ble_gap_adv_set_configure() routine, however, as we can't directly supply a RawAdvertisement with None for the scan_data field, but only the ScannableUndirected which requires a valid slice.

I get the impression using .as_ptr() to get an address for an empty slice may produce bogus values that are never meant to be used, such as 0x1, or really anything other than a NULL. It seems possible that the SoftDevice is applying some checks to those addresses before noticing that the length is zero. Possibly it insists that it be a valid RAM or flash address, or above its own region of memory. In any case, it seems that it may be a latent bug if we ever supply an address to the SoftDevice produced by as_ptr() on an empty slice, so I would suggest possibly the map_data() closure in start_adv() should check the length first, and always supply ptr::null_mut() as the address if the length is zero. Does that make any sense?

@peter9477
Copy link
Author

A better workaround is to do it as I just saw some examples do, with a static SCAN_DATA: [u8; 0] = [];.

This appears to work because it provides an address in flash memory. If you use const SCAN_DATA: [u8; 0] = []; instead, the as_ptr() produces a 1, which since it's definitely not a valid place for actual data seems to be why the SoftDevice complains about it.

@peter9477
Copy link
Author

Long story short: despite the workaround appearing to work fine, I'd propose changing map_data() to check the length and substitute an explicit null_ptr() in the event the length is zero. I believe that will avoid the issue in all cases, with no downsides.

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

No branches or pull requests

1 participant