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

Battery level related changes #1242

Merged
merged 3 commits into from
Feb 21, 2024
Merged

Battery level related changes #1242

merged 3 commits into from
Feb 21, 2024

Conversation

svillar
Copy link
Member

@svillar svillar commented Feb 8, 2024

This PR is made of 3 changes:

  1. Remove an unneccessary call to setbatterylevel in Vision Glass system (there are not controllers apart from the phone)
  2. Rename a misspelled method name
  3. Use a different battery icon tooltip for tethered devices (phone instead of headset)

This requires new translations

@svillar svillar requested a review from felipeerias February 8, 2024 13:37
@svillar
Copy link
Member Author

svillar commented Feb 15, 2024

Ping reviewers

Copy link
Collaborator

@felipeerias felipeerias left a comment

Choose a reason for hiding this comment

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

LGTM.

That method is used to report controllers' battery levels to Java
so that their values could be shown in the tray bar. In the case
of the Vision Glass the controller is at the same time the VR
system (i.e. the phone) so it does not make sense to report any
value there.

The battery level of the phone is already retrieved from the
Java side.
It lacks a "y", it should be named updateBatteryLevels.
When using tethered devices instead of standalone headsets, the battery
level we show in the tray is the phone's so it does make sense to
use "Phone" instead of "Headset".
@svillar svillar merged commit 5803399 into main Feb 21, 2024
20 checks passed
@svillar svillar deleted the vg_battery_level branch February 21, 2024 11:28
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.

2 participants