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

aioble: Pass additional connection arguments to gap_connect. #906

Merged
merged 1 commit into from
Oct 18, 2024

Conversation

Joris-van-der-Wel
Copy link
Contributor

In my project I am connecting using aioble to a human input device. There is noticeable input lag. Turns out the RTT latency is a little over 200ms (I test this by timing await char.write(b'foo', response=1)). I suspect this is because the BLE connection interval is being set to 100ms. The gap_connect function allows setting min_conn_interval_us and max_conn_interval_us, however there is no way to do so using aioble.

This PR allows the following additional arguments to be passed to device.connect():

  • scan_duration_ms
  • min_conn_interval_us
  • max_conn_interval_us

These are passed as-is to gap_connect(). The default value for all of these is None, which causes gap_connect to use its own defaults.

With this change I managed to get the RTT latency down to ~60ms, this feels a lot better in my project.

Package: https://joris-van-der-wel.github.io/micropython-lib/mip/gap_connect_params/

@ekspla
Copy link

ekspla commented Sep 9, 2024

I personally have no objection against this PR related to conn_intervals.

However, I sometimes saw an error when using characteristic.notified() with the arguments as followings.
I am not sure about the other possible errors that might be introduced by using these arguments.

scan_duration_ms=5000 or 2000
min_conn_interval_us=7500 or 11250
max_conn_interval_us=7500 or 11250

BOARD: ESP32-S3-WROOM-1
FIRMWARE: MPY-1.23.0 and also MPY-1.24.0-preview

Important part of the error trace:

  File "/lib/aioble/client.py", line 384, in notified
  File "/lib/aioble/client.py", line 378, in _notified_indicated
IndexError: empty

A fix for me to this error was to simply add asyncio.sleep_ms(5) just before reading the notified queue in client.py .
async def _notified_indicated(self, queue, event, timeout_ms):

    async def _notified_indicated(self, queue, event, timeout_ms):
        # Ensure that events for this connection can route to this characteristic.
        self._register_with_connection()

        # If the queue is empty, then we need to wait. However, if the queue
        # has a single item, we also need to do a no-op wait in order to
        # clear the event flag (because the queue will become empty and
        # therefore the event should be cleared).
        if len(queue) <= 1:
            with self._connection().timeout(timeout_ms):
                await event.wait()

        # Either we started > 1 item, or the wait completed successfully, return
        # the front of the queue.
+       await asyncio.sleep_ms(5)
        return queue.popleft()

@dpgeorge
Copy link
Member

Thanks @Joris-van-der-Wel for the contribution. This looks like a good enhancement to me. You definitely need to lower the connection interval to improve latency, and that should be possible to do in aioble (and this PR enables that).

@ekspla this PR does not change any of the defaults, it's an opt-in feature. For your case, it looks like a possible bug which needs to be discussed separately.

This allows the following arguments to be passed to `device.connect()`:

* scan_duration_ms
* min_conn_interval_us
* max_conn_interval_us

These are passed as-is to `gap_connect()`.  The default value for all of
these is `None`, which causes gap_connect to use its own defaults.

Signed-off-by: Joris van der Wel <[email protected]>
@dpgeorge dpgeorge merged commit 68e3e07 into micropython:master Oct 18, 2024
4 checks passed
@ekspla
Copy link

ekspla commented Nov 1, 2024

@dpgeorge Apologies. After tinkering my code, the issue I wrote in my previous comment was found to be not caused by AIOBLE, but caused by an error in handling a successive burst of notified data in the notified queue.

The reason was that the event was not cleared after reading data in the queue from another coroutine. A better fix in my case was to use characteristic._notify_event.clear() explicitly after reading in my code.

Though I am not sure what is the better way to handle a burst of successive notified packets using AIOBLE, the working server/client POC code for Nordic UART service is shown.

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.

3 participants