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

Missing Notification Sent While Subscribing Reproducing After Fix #130

Open
WkGl001 opened this issue Mar 14, 2024 · 4 comments
Open

Missing Notification Sent While Subscribing Reproducing After Fix #130

WkGl001 opened this issue Mar 14, 2024 · 4 comments
Labels
bug Something isn't working

Comments

@WkGl001
Copy link

WkGl001 commented Mar 14, 2024

Hello,

I filed this issue back in November but I'm only now updating the library and trying out the fix. After undoing a workaround for the issue on my end and trying it with version 1.0.15 of the library, I'm finding that this still does reproduce.

The fix in the library involved adding parameters for a buffer and applying one to the notifications flow:

return _notifications
            .apply { if (bufferSize > 0) buffer(bufferSize, bufferOverflow) }

from ClientBleGattCharacteristic.kt starting at line 139.

I'm still seeing the first notification being missed after subscribing when it is being sent in reaction to the subscription.
The buffer was added, which I could expect to get the initial notification, but it seems like, when tryEmit is called on _notifications, that does not happen. Looking at the doc for tryEmit, I see it say:

If there are no subscribers, the buffer is not used. Instead, the most recently emitted value is simply stored into the replay cache if one was configured, displacing the older elements there, or dropped if no replay cache was configured. In any case, tryEmit returns true.

I've tried to recreate a toy example of the differences in behavior here.
This uses a MutableSharedFlow set up similarly to the one for _notifications. There's one with no replay & no applied buffer, one with no replay but it does have an applied buffer, and one which does have a replay.
When run, the output on my end shows that the first tryEmit called directly after launching a coroutine to collect does not get collected for the first two MutableSharedFlows without a replay. The one with a replay does get collected. If I uncomment the delays before the first tryEmit calls, then all values emitted get collected.

Thank you.

@philips77
Copy link
Member

Hello,
I'm doing some refactoring in the project. I'll have a look at the issue when I'm done.

@philips77 philips77 added the bug Something isn't working label Mar 18, 2024
@juliankotrba
Copy link

@philips77 we are also impacted by this issue. It totally makes sense to postpone the bug fixing after the refactoring but can you already estimate when the new version of this library will be ready?

@philips77
Copy link
Member

Hello,
Reg your issue, I think adding replayCache isn't the right solution. That would reply the last notification to any new subscriber, am I right?
Instead, the flow should be created and set up for notifications before the notifications get enabled, so that the first one isn't missed. I'm not sure how this is done now in 1.0.15.

Regarding your question. I wrote my current progress here. Unfortunately, I had to switch to nRF Connect Device Manager app for some time to add a feature there, but I'll try working on both libs in parallel.

I believe I can try to find the issue on the current version before I'm ready with version 2.0.

@juliankotrba
Copy link

@philips77 thanks for the quick response! Regarding the replay cache, you are right!
A shared flow keeps a specific number of the most recent values in its replay cache. Every new subscriber first gets the values from the replay cache and then gets new emitted values. Source

When you say you believe you can find the issue, does that mean we can expect a new version before 2.0? If yes, I would really appreciate that and I think others would too.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants