Skip to content

Convert to use Android5.0 API for BluetoothLE scan#1329

Merged
jonasoreland merged 6 commits intomasterfrom
hr-ble-50
Mar 7, 2026
Merged

Convert to use Android5.0 API for BluetoothLE scan#1329
jonasoreland merged 6 commits intomasterfrom
hr-ble-50

Conversation

@jonasoreland
Copy link
Owner

@jonasoreland jonasoreland commented Mar 1, 2026

No description provided.

@jonasoreland
Copy link
Owner Author

jonasoreland commented Mar 1, 2026

This is a NOP change.
Only move to a more modern API

@jonasoreland
Copy link
Owner Author

jonasoreland commented Mar 1, 2026

I assume there is a lint suppression somewhere...but I couldn't find it.
Note this change will not improve reconnect behavior...that I'll continue on in other branch

@gerhardol
Copy link
Collaborator

gerhardol commented Mar 1, 2026

@copilot review

Copy link
Contributor

Copilot AI commented Mar 1, 2026

@gerhardol I've opened a new pull request, #1330, to work on those changes. Once the pull request is ready, I'll request review from you.

Copilot AI mentioned this pull request Mar 1, 2026
Closed
@gerhardol
Copy link
Collaborator

gerhardol commented Mar 1, 2026

Brief look, will try it out later this week.
Look kind of what I played around in an earlier attempt.

@gerhardol
Copy link
Collaborator

gerhardol commented Mar 1, 2026

I assume there is a lint suppression somewhere...but I couldn't find it. Note this change will not improve reconnect behavior...that I'll continue on in other branch

if you remove the lint-baseline.xml files, the new items will be added.
In this case, add checks as on old line 487.

@gerhardol
Copy link
Collaborator

gerhardol commented Mar 1, 2026

@jonasoreland copilot's comments should be included

@jonasoreland jonasoreland requested a review from gerhardol March 1, 2026 22:10
gerhardol
gerhardol reviewed Mar 2, 2026
View reviewed changes
Copy link
Collaborator

@gerhardol gerhardol left a comment

Choose a reason for hiding this comment

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

Tried it with #1330
Connects as expected to my generic ble 5.0 hr belt.

My Garmin Forerunner 255 in transmitt mode fails though. Also in play version, so not new. I believe this used to work.

gerhardol
gerhardol reviewed Mar 4, 2026
View reviewed changes
Copy link
Collaborator

@gerhardol gerhardol left a comment

Choose a reason for hiding this comment

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

Tested briefly on Android 16 and 14, seem to work the same.
Have not looked at why Garmin do not connect
Add the copilot commit too

hrdevice/src/org/runnerup/hr/AndroidBLEHRProvider.java
log("Ignoring paired non BLE device: " + btDeviceThis.getName());
continue;
// Paired device, for instance Huami/Amazfit Bip S could be supported
log("Trying paired generic BLE device: " + btDeviceThis.getName());
Copy link
Collaborator

@gerhardol gerhardol Mar 4, 2026

Choose a reason for hiding this comment

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

We should have someone to test these devices...

@jonasoreland
Copy link
Owner Author

jonasoreland commented Mar 7, 2026

This is now ready I think.
PTAL

  1. can we get copilot to make comments on this PR rather than creating a new??
  2. how to kill other PR it created ?

@gerhardol
Copy link
Collaborator

gerhardol commented Mar 7, 2026

just merge the copilot pr to this PR, it is targeting this branch.
I am approving with that PR

@jonasoreland
Copy link
Owner Author

jonasoreland commented Mar 7, 2026 •
edited
Loading

just merge the copilot pr to this PR, it is targeting this branch. I am approving with that PR

it was not great, but I included the "main" comment...
I made more changes.
I suggest we kill it instead.

@jonasoreland
Copy link
Owner Author

jonasoreland commented Mar 7, 2026 •
edited
Loading

also, can I get copilot to comment on this pr ?
it ignored my comment "@copilot review"...why ?
it that tied to your account ?

found this: "Copilot coding agent is available with the GitHub Copilot Pro, GitHub Copilot Pro+, GitHub Copilot Business and GitHub Copilot Enterprise plans"

@jonasoreland
Copy link
Owner Author

jonasoreland commented Mar 7, 2026

@copilot review

1 similar comment
@gerhardol
Copy link
Collaborator

gerhardol commented Mar 7, 2026

@copilot review

Copy link
Contributor

Copilot AI commented Mar 7, 2026

@gerhardol I've opened a new pull request, #1331, to work on those changes. Once the pull request is ready, I'll request review from you.

Copilot AI mentioned this pull request Mar 7, 2026
Merged
gerhardol
gerhardol approved these changes Mar 7, 2026
View reviewed changes
Copy link
Collaborator

@gerhardol gerhardol left a comment

Choose a reason for hiding this comment

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

Suggesting the minor comments in #1331 is included (one seem like an error.
Have not run the latest.
Can you please squash when merging, I find it hard to see the changes when looking at the history?

@gerhardol
Fix typo in mNotificationsOn and wrong log string in discoverServices…
6b9b1ef 
…() (#1331)

* Initial plan

* Fix typo mNotifictionsOn→mNotificationsOn and wrong log string in discoverServices()

Co-authored-by: gerhardol <6248932+gerhardol@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: gerhardol <6248932+gerhardol@users.noreply.github.com>
@jonasoreland jonasoreland merged commit 29ae6d3 into master Mar 7, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gerhardol gerhardol gerhardol approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@jonasoreland @gerhardol