Add counters to iskra integration #126046
Conversation
home-assistant
bot
added
cla-signed
integration: iskra
new-feature
Quality Scale: No score
labels
|
Hey there @Iskramis, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
| @@ -7,5 +7,5 @@ | |||
| "integration_type": "hub", | |||
| "iot_class": "local_polling", | |||
| "loggers": ["pyiskra"], | |||
| "requirements": ["pyiskra==0.1.8"] | |||
| "requirements": ["pyiskra==0.1.11"] | |||
|
That PR can be a PR that we merge before this PR. Separating dependency bumps has a lot of benefits. :) |
|
will do as suggested :P |
| @@ -163,6 +167,46 @@ class IskraSensorEntityDescription(SensorEntityDescription): | |||
| ) | |||
|
|
|||
|
|
|||
| def get_counter_sensor( | |||
There was a problem hiding this comment.
Can we refactor this to return a EntityDescription?
There was a problem hiding this comment.
for sure, will do
| sensor_description = { | ||
| "key": key, | ||
| "translation_key": key, | ||
| "state_class": SensorStateClass.TOTAL_INCREASING, | ||
| "native_unit_of_measurement": counter.units, | ||
| } | ||
|
|
||
| if entity_name == ATTR_NON_RESETTABLE_COUNTER: | ||
| sensor_description.update( | ||
| {"value_func": lambda device: device.counters.non_resettable[index].value} | ||
| ) | ||
| else: | ||
| sensor_description.update( | ||
| {"value_func": lambda device: device.counters.resettable[index].value} | ||
| ) |
There was a problem hiding this comment.
I think this part should be like
if ...:
entity_description = IskraSensorEntityDescription()
else:
entity_description = IskraSensorEntityDescription()and the thing down below can use .replace to add the UoM and device class
WDYT?
There was a problem hiding this comment.
with .replace you meant like that?
entity_description.native_unit_of_measurement = UnitOfEnergy.WATT_HOUR
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
MartinHjelmare
changed the title
| key=key, | ||
| translation_key=key, | ||
| state_class=SensorStateClass.TOTAL_INCREASING, | ||
| value_func=(lambda device: device.counters.non_resettable[index].value), |
There was a problem hiding this comment.
you only have to parenthesize lambdas if they span more than one line
| @@ -205,6 +247,19 @@ async def async_setup_entry( | |||
| if description.key in sensors | |||
| ) | |||
|
|
|||
| if device.supports_counters: | |||
| for index, counter in enumerate(device.counters.non_resettable[:4]): | |||
There was a problem hiding this comment.
unique_id is assembled in entity.py using device serial and entity description key. entity description key is ATTR_RESETTABLE_COUNTER or ATTR_NON_RESETTABLE_COUNTER with added counter index
In line 177 in sensor.py (get_counter_entity_description) i use .format to add index
There was a problem hiding this comment.
i hope this is OK.
There was a problem hiding this comment.
Can the counter change? We want to avoid setting up and having the data point that previously represented counter 1 to be counter 2 and vice versa
There was a problem hiding this comment.
they cannot those counters are directly from the energy meter's registers, always in same order
Proposed change
This is a follow up PR to #121488
It adds Energy counter sensor entities for displaying consumption
Type of change
Additional information
Checklist
ruff format homeassistant tests)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest.requirements_all.txt.Updated by running
python3 -m script.gen_requirements_all.To help with the load of incoming pull requests: