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

Allow processing soc events and ble events in separate tasks #189

Merged
merged 1 commit into from
Jul 29, 2023

Conversation

lulf
Copy link
Member

@lulf lulf commented Jul 25, 2023

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)

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.
@lulf lulf requested a review from alexmoon July 26, 2023 13:16
@alexmoon
Copy link
Contributor

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?

@lulf
Copy link
Member Author

lulf commented Jul 27, 2023

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.

@lulf lulf added this pull request to the merge queue Jul 29, 2023
Merged via the queue into master with commit 6415c63 Jul 29, 2023
2 checks passed
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.

2 participants