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

Bluetooth Devices disconnect upon bluetooth sensor switch #405

Draft
wants to merge 8 commits into
base: develop
Choose a base branch
from

Conversation

posytive
Copy link
Contributor

Fixes #404

Draft because:

  • some tests need to be fixed
  • some tests need to be added
  • mocks need to be improved (to simulate bt on/off scenarios)
  • (optional) mock can be moved to a different module

Copy link
Contributor

@dasdranagon dasdranagon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For me looks fine. I would only remove commented code

@codecov
Copy link

codecov bot commented Oct 27, 2021

Codecov Report

Merging #405 (3a9557c) into develop (b6234ec) will decrease coverage by 0.27%.
The diff coverage is 47.22%.

Impacted file tree graph

@@              Coverage Diff              @@
##             develop     #405      +/-   ##
=============================================
- Coverage      48.38%   48.11%   -0.28%     
  Complexity       219      219              
=============================================
  Files             62       62              
  Lines           2412     2436      +24     
  Branches         310      318       +8     
=============================================
+ Hits            1167     1172       +5     
- Misses          1108     1126      +18     
- Partials         137      138       +1     
Impacted Files Coverage Δ
...etooth/src/commonMain/kotlin/device/DeviceState.kt 70.94% <47.22%> (-7.76%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b6234ec...3a9557c. Read the comment docs.

remainIfStateNot = Disconnected::class
) { deviceState ->
connect(deviceState)
data class WaitingForBluetooth constructor(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is something to discuss. This state represents Device when Bluetooth is turned off by the system. On iOS it means all services/characteristic objects are invalid (we can't read/write from them anymore). Also as far as I know Identifier for Bluetooth peripheral autogenerated by system at the time of discovery (compare to Android, where it's always MAC address of the device). We have to check if keeping Identifier somewhere and doing connection/reconnection/bluetooth/on/off after next scan we can access to the same device by this Identifier.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, I haven't thought of that cause I din't know about this requirement for ios.
Is true that I have been having re-connect issues on iOs for a project using this lib, at this specific branch, but I was blaming a different part of the code for it.
(we disabled automatic reconnection on that project, due to un-stable behaviour in general)
Interesting to know: I should now check this again, following on what you pointed out.

In my understanding, was enough to make sure Not to save services/characteristics the same way the original code was already doing upon other types of disconnection.
You can see in this file, at line 346, that internal val noBluetooth = suspend {} function passes a null for those.
In my understanding, that should have been enough to guarantee that re-connecting would fetch services/characteristics again.

Long story short, I will check the feasibility/impact of keeping Identifier in the state.
Is a bit unfortunate that I don't have with me the device we use on the other project, but I will check what I can do using our own example.

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.

Provide Disconnected State when bluetooth is off, delay reconnecting strategy
4 participants