-
Notifications
You must be signed in to change notification settings - Fork 2.1k
sys/stdio_nimble: Send Callout called indefinitely even without anything to send #21192
Description
Description
When data is sent via stdio_nimble the _write function is called, which creates a so called _send_stdout_callout https://github.com/RIOT-OS/RIOT/blob/master/sys/stdio_nimble/stdio_nimble.c#L322-L327.
This callout then calls the internal function _send_stdout, which actually does the heavy lifting of getting the data out via BLE.
In _send_stdout the callout is rearmed, so it will be called again after the timeout of 1ms has elapsed. https://github.com/RIOT-OS/RIOT/blob/master/sys/stdio_nimble/stdio_nimble.c#L188-L189
I guess the reason for this is to make sure all data in the stdout ringbuffer is actually being sent, when the data size is greater than NIMBLE_MAX_PAYLOAD. After the first transmission, the _status state will change from STDIO_NIMBLE_SENDING back to STDIO_NIMBLE_SUBSCRIBED and the next BLE package is sent until no data is left in the stdout buffer.
Now here's the catch: _send_stdout_callout is never disarmed. Even when there's nothing to send, it will be called every millisecond and create CPU load. This behavior has been present since the PR #12012 was merged.
My proposal would be to move the rearming into the if (to_send > 0) statement https://github.com/RIOT-OS/RIOT/blob/master/sys/stdio_nimble/stdio_nimble.c#L195 . Only if there's something to send, we would even have the need to rearm the callout. Even better would be if there is a condition such as if (to_send > NIMBLE_MAX_PAYLOAD)if (tsrb_avail(&_tsrb_stdout) > NIMBLE_MAX_PAYLOAD) to rearm the function, because if everything has been sent, there is no need to rearm the callout in the first place.
I haven't tested this yet because until writing this down, I was not confident enough that I understood the mechanics of stdio_nimble enough. So comments on this are very welcome!
I hope @HendrikVE is still active and maybe can give some input on this.
Versions
From the merge of #12012 to latest master.