Don't forget previously discovered characteristics #747
Conversation
Without losing previously discovered characteristics. Otherwise, every call to discoverCharacteristics forgets any previously discovered characteristic that is not included in the most recent call.
…verCharacteristics multiple times
only characteristics.
|
These are my changes. I will include follow-up commits that roll in the fixes described in the suggestions on #730 once I have tested those. |
|
@jacobrosenthal tested this overnight with multiple connects/disconnects and discoveries, as well as multiple devices connected at the same time, all working fine. This removes a lot of irritating crashes that occur when using noble intensively. |
|
What about that early exit PR https://github.com/sandeepmistry/noble/pull/383/files , that seemed kind of a classy way to do it. Can you run that and see if it similarly works well? |
|
I will check #383 - if it works, that would be neat, definitely. |
|
OK, just checked, with #383 on its own, the code crashes again at it did earlier - that PR might have solved edge cases, but it doesn't seem to provide a complete solution like this PR does. Let me know if you can try this PR on your side and test on your side, and whether it works for you, merging it would be a tremendous help. |
|
Did a few more tests - multiple connect/disconnect, multiple "discover" for services and characteristics in each session, this PR solves all the crashes or hangs I got earlier whereas the simple fix to "noble.js" in #383 really does not, I'm afraid. |
|
@elafargue thanks for updating, this looks good visually to me. We'll need to do some testing here before merging. |
|
Ill take a look with some of my existing code. @monteslu do we need anything for webble? @tripish @Chondrules does this hit your needs from your previous PR? |
|
I think we're good on the web ble bindings. The call to |
|
Last attempt to try to ping @tripish @Chondrules to see if this solves their old PRs #383 and #541 |
|
Thank you!
…On Feb 11, 2018 08:14, "Jacob Rosenthal" ***@***.***> wrote:
Merged #747 <#747>.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#747 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/ABH4B6UzFDvXPeaSMiUB5vZ-0dHTg9AZks5tTxIBgaJpZM4R0woR>
.
|
... when calling discoverCharacteristics multiple times in a program.
Without this fix, we get crashes when unsubscribing, and never receive 'data' events on previously subscribed characteristics if calling discoverCharacteristics multiple times with different uuid lists.
This PR includes comments/feedback from #730