Closed lulf closed 1 year ago
Looks good to me. I don't think there should be any issues with splitting the SoC and BLE event handling into separate executors (as long as they're both at sufficiently low priority).
I am a little curious about your use case though. What BLE event are you handling that needs to do a flash write before returning? The way I handled the same problem for peripheral pairing was to have the BLE event handler pass a completion object that could be sent to a separate task for async handling. Perhaps there are other BLE events that could use similar treatment?
My specific use case is implementing the nRF DFU protocol. My initial approach was as you suggest to dispatch the event to a separate task. However, there is no flow control implemented in the protocol, so when the event is dispatched to a queue/channel for processing later, the next event from the DFU app will follow immediately because there is no longer any back-pressure.
One solution I found semi-working was to size the processing channel sufficiently big, but this turns out to consume to much memory. Also, this complicates error handling.
The other solution is to block on flash operations in the event handler, ensuring proper flow control. However, that caused the issue that this PR is trying to solve, handling flash operation completion events at a higher priority in order for the event handler to make progress writing to flash.
This change splits the run function into two: one for processing BLE events and one for processing SoC events, while keeping the existing API joining the two futures.
With this change, GATT server closures, which are invoked by the ble event task, can use the flash peripheral successfully (which relies on soc events for completion) by running the soc event task on a separate executor (although I'm not entirely sure if that's safe! It seem to work)