Remove duplicate code in emoncms #118610
Remove duplicate code in emoncms #118610
Conversation
home-assistant
bot
added
cla-signed
code-quality
integration: emoncms
small-pr
|
Hey there @borpin, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
MartinHjelmare
changed the title
| if self._value_template is not None: | ||
| self._attr_native_value = ( | ||
| self._value_template.render_with_possible_json_value( | ||
| elem["value"], STATE_UNKNOWN | ||
| ) | ||
| ) | ||
| elif elem["value"] is not None: | ||
| self._attr_native_value = round(float(elem["value"]), DECIMALS) | ||
| else: | ||
| self._attr_native_value = None |
There was a problem hiding this comment.
I think this is still needed to get the initial value (on start).
Otherwise I think it will get set as "unknown" until the first data update.
In which case, if you want to remove duplicate code, maybe you can add a function _update_attributes that gets called in both __init__ and update
There was a problem hiding this comment.
I thought to call self.update() at the end of the __init__ of the EmonCmsSensor class
is it OK for you ?
For an upcoming PR, I think also :
- there is not benefit to have feedtag and feedname in the identifier,
- there is one to have feed tag in the friendly name (
self._attr_name) as users will identify more easily the feeds - the code in the
__init__which fixesself._attr_namecould be in the update method, because if a user modify a tag or a feed name, it will be reported in home assistant, which is not the case with the actual implementation
There was a problem hiding this comment.
You shouldn't call self.update() inside __init__, as it has already been called once in setup_platform
This is why I think you should split get data and set attributes to avoid duplicate code
There was a problem hiding this comment.
in setup_platform, it is the update method of the EmonCmsData class which is called, that's why I thought it could be possible to call the update method of EmonCmsSensor in its __init__
when adding entities, update is not called by default to what I understand :
but no problem to refactor as you suggest, it is probably more flexible
|
@epenet - is it acceptable for there to be a 'timeout' before an entity is set to Unknown? The way emoncms works, using a timeseries database, a NULL return is acceptable, especially for a single value (if the incoming value misses the timeseries window). However, an unknown value, say for a temperature sensor does cause issues within HA (in automation and templates). Ideally, a 'timeout' whereby it is not immediately assumed the value is UNKNOWN, but just does not get updated if the incoming value is NULL. @alexandrecuer - perhaps the answer is not to update the sensor if the value is NULL. It is then clear to the user when it was last updated, but the value remains valid. |
That's a question for another PR. |
alexandrecuer
changed the title
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
Co-authored-by: epenet <6771947+epenet@users.noreply.github.com>
Co-authored-by: epenet <6771947+epenet@users.noreply.github.com>
Co-authored-by: epenet <6771947+epenet@users.noreply.github.com>
Co-authored-by: epenet <6771947+epenet@users.noreply.github.com>
* Remove duplicate & property extra_state_attributes * Add methods _update_attributes and _update_value * correction in _update_value * Update homeassistant/components/emoncms/sensor.py Co-authored-by: epenet <6771947+epenet@users.noreply.github.com> * Update homeassistant/components/emoncms/sensor.py Co-authored-by: epenet <6771947+epenet@users.noreply.github.com> * Update homeassistant/components/emoncms/sensor.py Co-authored-by: epenet <6771947+epenet@users.noreply.github.com> * Update homeassistant/components/emoncms/sensor.py Co-authored-by: epenet <6771947+epenet@users.noreply.github.com> --------- Co-authored-by: epenet <6771947+epenet@users.noreply.github.com>
Proposed change
Remove duplicate & property extra_state_attributes
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..coveragerc.To help with the load of incoming pull requests: