jblance / mpp-solar

Python package to communicate to MPP Solar PIP-4048MS inverters (and similar)
MIT License
364 stars 151 forks source link

Change "generated_energy" description to more specific #463

Closed JakobTewes closed 8 months ago

JakobTewes commented 8 months ago

Hi @jblance,

thanks for putting all the time and effort to this (once again :-)).

I´m using your solution since months and it gets better and better (for example the support for pi17m058).

Now I wanted to get even more info out of my inverters and added queries for generated energy:

image

I noticed two things, that maybe can be improved:

Thanks so much and regards

Jakob

JakobTewes commented 8 months ago

Some additional info on this:

Using this command: mpp-solar -p /dev/hidrawINV2 -n 'BPVInverter2' -T 'BPVInverter2' -P pi17m058 -o hass_mqtt --mqttbroker 1.2.3.4 --mqttuser bpvinverter --mqttpass mqttpass -c ET And would love, using mpp-solar -p /dev/hidrawINV2 -n 'BPVInverter2' -T 'BPVInverter2' -P pi17m058 -o hass_mqtt --mqttbroker 1.2.3.4 --mqttuser bpvinverter --mqttpass mqttpass -c ET#EDdate +"%Y%m%d"#EMdate +'%Y%m' -d 'last month'` Resulting in this mqtt topic:

image

And this Home Assistant entity:

image
jblance commented 8 months ago
  • First is the field naming. All the queries for generated energy (total, daily, monthly) are reported back as "generated_energy". For my scenario this is not ideal, as i ingest the data into Home Assistant, resulting in not shining up as "generated_energy" is reported for all three used queries. Can we (eh you ;-)) change this to for example "generated_energy_total", generated_energy_day", "generated_energy_month"?

I can do this, though you can 'simulate' this by usng 3 separate commands and changing the tag / or using the command to differentiate.

  • Second thing (and this I realised while creating this issue ;-)) is, that the inverter seems to report back additional data, that can´t be mapped (those "unknown_value_in_response_X"). Can you maybe map this and is there anything, that I can do to support you in mapping the data for the pi17m058 inverters?

I can, however I dont know what they are - any idea what the fields relate to? They kinda look like responses to something else sending commands?

JakobTewes commented 8 months ago
  • First is the field naming. All the queries for generated energy (total, daily, monthly) are reported back as "generated_energy". For my scenario this is not ideal, as i ingest the data into Home Assistant, resulting in not shining up as "generated_energy" is reported for all three used queries. Can we (eh you ;-)) change this to for example "generated_energy_total", generated_energy_day", "generated_energy_month"?

I can do this, though you can 'simulate' this by usng 3 separate commands and changing the tag / or using the command to differentiate. Great - thanks for this idea, will dive into it and give it a try. In the meantime, can you please consider whether you would like to incorporate this?

  • Second thing (and this I realised while creating this issue ;-)) is, that the inverter seems to report back additional data, that can´t be mapped (those "unknown_value_in_response_X"). Can you maybe map this and is there anything, that I can do to support you in mapping the data for the pi17m058 inverters?

I can, however I dont know what they are - any idea what the fields relate to? They kinda look like responses to something else sending commands? Can dig into that these days and provide ideas for mappings after consulting other queries or cheching on the device and via SolarPower. Would that help and do you think, its worth investing a few hours?

jblance commented 8 months ago
  • First is the field naming. All the queries for generated energy (total, daily, monthly) are reported back as "generated_energy". For my scenario this is not ideal, as i ingest the data into Home Assistant, resulting in not shining up as "generated_energy" is reported for all three used queries. Can we (eh you ;-)) change this to for example "generated_energy_total", generated_energy_day", "generated_energy_month"?

I can do this, though you can 'simulate' this by usng 3 separate commands and changing the tag / or using the command to differentiate. Great - thanks for this idea, will dive into it and give it a try. In the meantime, can you please consider whether you would like to incorporate this?

Should be in latest version

  • Second thing (and this I realised while creating this issue ;-)) is, that the inverter seems to report back additional data, that can´t be mapped (those "unknown_value_in_response_X"). Can you maybe map this and is there anything, that I can do to support you in mapping the data for the pi17m058 inverters?

I can, however I dont know what they are - any idea what the fields relate to? They kinda look like responses to something else sending commands? Can dig into that these days and provide ideas for mappings after consulting other queries or cheching on the device and via SolarPower. Would that help and do you think, its worth investing a few hours?

I think so, but I dont have these inverters so its more "is it of use to you" I'll incorporate any improvements you can find

JakobTewes commented 8 months ago
  • First is the field naming. All the queries for generated energy (total, daily, monthly) are reported back as "generated_energy". For my scenario this is not ideal, as i ingest the data into Home Assistant, resulting in not shining up as "generated_energy" is reported for all three used queries. Can we (eh you ;-)) change this to for example "generated_energy_total", generated_energy_day", "generated_energy_month"?

I can do this, though you can 'simulate' this by usng 3 separate commands and changing the tag / or using the command to differentiate. Great - thanks for this idea, will dive into it and give it a try. In the meantime, can you please consider whether you would like to incorporate this?

Should be in latest version

Can confirm - very nice. Two small thing regarding ordering and naming: I think, it would be better, having the time period added at the end instead of the beginning of the sensor name. Also as were returining the generated energy of a day or month, I think it would be good, having "day" and "month" instead of "daily" and "monthly". Would result in "generated_energy_month" instead of "monthly_generated_energy". What do you think?

  • Second thing (and this I realised while creating this issue ;-)) is, that the inverter seems to report back additional data, that can´t be mapped (those "unknown_value_in_response_X"). Can you maybe map this and is there anything, that I can do to support you in mapping the data for the pi17m058 inverters?

I can, however I dont know what they are - any idea what the fields relate to? They kinda look like responses to something else sending commands? Can dig into that these days and provide ideas for mappings after consulting other queries or cheching on the device and via SolarPower. Would that help and do you think, its worth investing a few hours?

I think so, but I dont have these inverters so its more "is it of use to you" I'll incorporate any improvements you can find

Purrfect....will dig into that as soon as i find some minutes ;-)

jblance commented 8 months ago

Can confirm - very nice. Two small thing regarding ordering and naming: I think, it would be better, having the time period added at the end instead of the beginning of the sensor name. Also as were returining the generated energy of a day or month, I think it would be good, having "day" and "month" instead of "daily" and "monthly". Would result in "generated_energy_month" instead of "monthly_generated_energy". What do you think?

Ive made this change

JakobTewes commented 8 months ago

Can confirm - very nice. Two small thing regarding ordering and naming: I think, it would be better, having the time period added at the end instead of the beginning of the sensor name. Also as were returining the generated energy of a day or month, I think it would be good, having "day" and "month" instead of "daily" and "monthly". Would result in "generated_energy_month" instead of "monthly_generated_energy". What do you think?

Ive made this change

Did you push it? Updated this morning and still got the old names ;)

jblance commented 8 months ago

its updated in github, I havent pushed a package update yet

jblance commented 8 months ago

update pushed now 0.16.22+