pzemac total energy support #933
Conversation
esphome/const.py
Outdated
| @@ -523,6 +524,7 @@ | |||
| ICON_PERCENT = 'mdi:percent' | |||
| ICON_PERIODIC_TABLE_CO2 = 'mdi:periodic-table-co2' | |||
| ICON_POWER = 'mdi:power' | |||
| ICON_ENERGY = 'mdi:gauge' | |||
There was a problem hiding this comment.
These constants are named after their content (and specifically do not mandate any semantic meaning) - so it should be called ICON_GAUGE (which already exists).
But I don't think the gauge icon is good here (total energy is usually not read with a gauge, since it's an accumulated value). I would prefer ICON_FLASH or ICON_CURRENT_ACT
There was a problem hiding this comment.
@OttoWinter are you agree with ICON_COUNTER?
esphome/const.py
Outdated
| @@ -340,6 +340,7 @@ | |||
| CONF_POSITION = 'position' | |||
| CONF_POSITION_ACTION = 'position_action' | |||
| CONF_POWER = 'power' | |||
| CONF_ENERGY = 'energy' | |||
There was a problem hiding this comment.
See the lint error in the travis CI log - these constants are in alphabetical order (so you need to put it to the appropriate position)
|
I have modified the @yekm pull request to make the linter happy. I have fixed the icon issue choosing the counter icon, if @OttoWinter agrees: yekm#1 Please @yekm accept this pull request in order to get this fantastic feature ASAP. Regards :-) |
|
Yes please!! , I have been waiting for this functionality for a long time, it would be incredible to have it available in the next version. |
Fix lint requested by Ottowinter
|
@OttoWinter evrything looks fine for me. Give some attention to us please. |
esphome/const.py
Outdated
| @@ -550,6 +551,7 @@ | |||
| UNIT_DEGREE_PER_SECOND = '°/s' | |||
| UNIT_DEGREES = '°' | |||
| UNIT_EMPTY = '' | |||
| UNIT_ENERGY = 'Wh' | |||
There was a problem hiding this comment.
This needs to be UNIT_WATT_HOUR (all these constants are named after the unit name, not the name of the quantity, see for example W)
|
LGTM @yekm |
|
LGTM |
|
Hi, Why the Energy addin is not yet pushed ?. I think it's very usefull ..... |
We are waiting for an admin to merge this PR. |
It's too long :) :))))) @OttoWinter can you hear us ? ;) |
|
Thanks @brandond!! :-) |
OttoWinter
added
component: sensor.pzemac
core
has-tests
integration: pzemac
and removed
component: sensor.pzemac
labels
Description:
Related feature request esphome/feature-requests#457
Checklist:
The code change is tested and works locally.
Tests have been added to verify that the new code works (under
tests/folder).Documentation added/updated in esphome-docs add energy support to pzemac esphome-docs#456