Skip to content

Add a seperate battery_state sensor#530

Merged
JBassett merged 5 commits into
home-assistant:masterfrom
jeroenseegers:battery-state-sensor
Mar 29, 2020
Merged

Add a seperate battery_state sensor#530
JBassett merged 5 commits into
home-assistant:masterfrom
jeroenseegers:battery-state-sensor

Conversation

@jeroenseegers
Copy link
Copy Markdown
Contributor

As proposed in #529, this PR adds a battery_state sensor.

The current BatterySensor enables a single battery_level sensor which contains different battery data, example:
Screenshot 2020-03-27 at 14 30 21

With this change, there's now two sensors, battery_level and battery_state, similar to the sensors available when using the iOS application, example:
Screenshot 2020-03-27 at 14 32 46

@homeassistant
Copy link
Copy Markdown
Contributor

Hi @jeroenseegers,

It seems you haven't yet signed a CLA. Please do so here.

Once you do that we will be able to review and accept this pull request.

Thanks!

percent,
percentage,
"sensor",
getBatteryIcon(batteryStep),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why not display the fancy battery icon for both sensors?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I figured since this sensor is now purely for the battery level, the icon should also represent just that.

@quthla
Copy link
Copy Markdown
Contributor

quthla commented Mar 27, 2020

It seems most people don't need this and for those who do, can't they just make a template sensor of the attribute and have the same effect?

@dshokouhi
Copy link
Copy Markdown
Member

@quthla this will actually help bring iOS and Android closer in terms of features. iOS already splits up the 2 sensors for users. https://companion.home-assistant.io/docs/core/sensors/#battery-sensors

@jeroenseegers
Copy link
Copy Markdown
Contributor Author

jeroenseegers commented Mar 27, 2020

Correct, I figured it'd be very nice from an architectural point of view to have the same sensors and their respective properties to be published from both the iOS and Android companion apps.

The other option to achieve that would be to remove and combine sensors on the iOS side, ofcourse.

@jeroenseegers jeroenseegers requested a review from JBassett March 29, 2020 09:33
@JBassett JBassett merged commit 4a9c6f0 into home-assistant:master Mar 29, 2020
@jeroenseegers jeroenseegers deleted the battery-state-sensor branch March 29, 2020 13:18
jeroenseegers added a commit to jeroenseegers/companion.home-assistant that referenced this pull request Mar 29, 2020
Update the documentation according to the merged PR: home-assistant/android#530
TomBrien added a commit to home-assistant/companion.home-assistant that referenced this pull request Mar 29, 2020
* Update sensors.md

Update the documentation according to the merged PR: home-assistant/android#530

* Update docs/core/sensors.md

Prevent iOS battery sensor descriptions from being split up.

Co-Authored-By: Tom Brien <TomBrien@users.noreply.github.com>

* Update docs/core/sensors.md

iOS Battery Sensor description merged into top paragraph.

Co-Authored-By: Tom Brien <TomBrien@users.noreply.github.com>

Co-authored-by: Tom Brien <TomBrien@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants