Convert solarlog to coordinator #55345
Conversation
|
Hey there @Ernst79, mind taking a look at this pull request as it has been labeled with an integration ( |
|
Yes, thanks. I noticed that it sends a
we have to change the first line I do notice that the error isn’t added to the log. If I add a manual |
|
Alright pushed a fix. It just looks if the year is 1999 and halts on that. |
|
|
||
| data = {} | ||
|
|
||
| try: |
There was a problem hiding this comment.
the pypi package seems to either send all data, or nothing {}, so I think we can do
if api:
data["TIME"] = api.time
data["powerAC"] = api.power_ac
data["powerDC"] = api.power_dc
data["voltageAC"] = api.voltage_ac
data["voltageDC"] = api.voltage_dc
data["yieldDAY"] = api.yield_day / 1000
data["yieldYESTERDAY"] = api.yield_yesterday / 1000
data["yieldMONTH"] = api.yield_month / 1000
data["yieldYEAR"] = api.yield_year / 1000
data["yieldTOTAL"] = api.yield_total / 1000
data["consumptionAC"] = api.consumption_ac
data["consumptionDAY"] = api.consumption_day / 1000
data["consumptionYESTERDAY"] = api.consumption_yesterday / 1000
data["consumptionMONTH"] = api.consumption_month / 1000
data["consumptionYEAR"] = api.consumption_year / 1000
data["consumptionTOTAL"] = api.consumption_total / 1000
data["totalPOWER"] = api.total_power
data["alternatorLOSS"] = api.alternator_loss
data["CAPACITY"] = round(api.capacity * 100, 0)
data["EFFICIENCY"] = round(api.efficiency * 100, 0)
data["powerAVAILABLE"] = api.power_available
data["USAGE"] = round(api.usage * 100, 0)
if api.time.year == 1999:
raise update_coordinator.UpdateFailed(
"Invalid data returned (can happen after Solarlog restart)"
)
else:
raise update_coordinator.UpdateFailed(
"Solarlog response returns no data"
)There was a problem hiding this comment.
Sorry, just posted the above comment while you added your last commit. Modified it.
Thanks for your help.
There was a problem hiding this comment.
I don't think that will work because api is a SolarLog object
There was a problem hiding this comment.
If the current code works I suggest we go with that and you can clean it up in a future PR. Since I don't have a device to test it with, I don't want to do more than necessary.
|
The only thing that is strange is that I NEVER see an error in the HA logs about "Invalid data returned (can happen after Solarlog restart)". When I add a It also does seem to halt on the raised exception, as I don't get the zero values in the graphs and the sensors become unavailable for a few minutes. But, I was expecting that it logs the error in the HA log as well. Not very important to fix now. I can look into that later myself. |
| @property | ||
| def native_value(self) -> StateType: | ||
| """Return the native sensor value.""" | ||
| return self.coordinator.data[self.entity_description.json_key] |
There was a problem hiding this comment.
Side note for the future: If we rename the keys in the data returned by the coordinator to the same names as the entity description keys, we can drop the json_key from the entity description and use the entity description key instead.
There was a problem hiding this comment.
Thanks, But that will cause a change in the unique_id. I don’t think that is desired.
self._attr_unique_id = f"{coordinator.unique_id}_{description.key}"
There was a problem hiding this comment.
No, we won't change the description key attribute just remove the json_key attribute.
There was a problem hiding this comment.
But I need the JSON key to get the correct value from the sunwatcher pypi package. This package is having partly capitalized works as keys in its output
There was a problem hiding this comment.
We don't use the json_key to get the data from the pypi library. We use it to get the data from the coordinator, which is data we define in this integration. We can define that data dict however we want.
There was a problem hiding this comment.
Ah, sorry, you are totally right. I’ll add that in a future PR
Breaking change
Proposed change
This converts the solarlog integration to use a data update coordinator. I did not test this PR as I don't have a solarlog device.
@Ernst79 can you test this?
Fixes #55254
There is still room for improvement to this integration I think, like storing the SolarLog object directly instead of converting it to a dict and moving the calculations into the
SolarLogSensorEntityDescriptionobject. But that I leave for someone else :)Type of change
Additional information
Checklist
black --fast 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.The integration reached or maintains the following Integration Quality Scale:
To help with the load of incoming pull requests: