Closed chrysn closed 1 year ago
This approach seems ok to me. However, I'd like the default to stay as it currently is (128 bytes). Perhaps two features for 256, and 512 bytes and the size is picked based on the largest activated feature (so the features are properly additive)?
Quite frankly, that's a complete oversight on my part -- I doubled it during early experimentation, always only compared the original branch and "the new one with the feature enabled", and just missed that I doubled twice. Two features would indeed be the way to go, will update.
Fixed, and squashed into a nice sequence again. Now there are -256 and -512 features. The cfg macros became a bit unwieldy (esp. because features are additive -- if a dependency says 256 and one says 512, 512 it is without a conflict), but I don't think it warrants pulling in that one macro crate that provides it just to make two lines a bit easier to read.
Thanks!
The recognized error in sd_ble_evt_get was not the one that's raised when BLE_EVT_MAX_SIZE is exceeded -- fixed in a first commit.
The larger issue is that BLE_EVT_MAX_SIZE (allocated on the stack) really depends on a run-time variable. Introducing a feature to increase it is a workaround that'd help users not have to fork softdevice just to increase the limit. 512 was chosen arbitrarily, or by "half or double" rule. If larger MTUs even make sense (don't know off my head), a 1024 might make sense as well.
Potential further improvements: