Some hybrid inverters start with SD not SA

[zaheerm]

No description provided.

[dewet22]

Sorry, been traveling so only getting around to this now! Good to know about this - out of curiosity, what is the text description of your model in the GE dashboard? Mine's Giv-HY5.0

[codecov]

Codecov Report

Merging #6 (0c92b35) into main (db0cf5c) will increase coverage by 0.05%.
The diff coverage is 100.00%.

❗ Current head 0c92b35 differs from pull request most recent head 29ed4a2. Consider uploading reports for the commit 29ed4a2 to get more accurate results

@@            Coverage Diff             @@
##             main       #6      +/-   ##
==========================================
+ Coverage   90.05%   90.10%   +0.05%     
==========================================
  Files          17       17              
  Lines        1659     1668       +9     
  Branches      153      155       +2     
==========================================
+ Hits         1494     1503       +9     
  Misses        142      142              
  Partials       23       23              

Impacted Files	Coverage Δ
givenergy_modbus/model/inverter.py	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 48b8d8b...29ed4a2. Read the comment docs.

[dewet22]

You should also do a tox run over the proposed changes (or install and run pre-commit) - the lint tests failed with:

  givenergy_modbus/model/inverter.py:14:1: D101 Missing docstring in public class
  givenergy_modbus/model/inverter.py:26:1: D102 Missing docstring in public method
  tests/model/test_inverter.py:348:1: D103 Missing docstring in public function

[zaheerm]

what is the text description of your model in the GE dashboard?

It's Giv-HY5.0 for me.

[dewet22]

Interesting, so they seem to be labeled identically. I took your contribution and reworked it a bit last night – hope you don't mind or feel I appropriated it without giving credit (definitely one for the CHANGELOG) but the JSON serialisation is better now (model name instead of serial number prefix) and I fixed the lint warnings at the same time.

[zaheerm]

Thanks no issues, I'm on holiday now so didn't get to the suggested linting. I read your changes and they make sense. In fact I started my changes, changing to auto() instead of keeping the strs. I just didn't know what implications it may have had elsewhere in any dependent code.

[dewet22]

It's Giv-HY5.0 for me.

Interesting – talking to the folks at GivEnergy they claim the SA model is the 3.6kW Hybrid inverter. Do you know which model you actually have?

[zaheerm]

I have a model with a serial number starting SD and the inverter is definitely a 5 kW inverter.

…

On Sun, 20 Feb 2022, 17:34 Dewet Diener, ***@***.***> wrote: It's Giv-HY5.0 for me. Interesting – talking to the folks at GivEnergy they claim the SA model is the 3.6kW Hybrid inverter. Do you know which model you actually have? — Reply to this email directly, view it on GitHub <#6 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAQ2T6RPORFBLGHVKBZBM3U4ECXPANCNFSM5NVTYSKQ> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[dewet22]

Ah, I got it the wrong way around - mine starts with SA but is also definitely a 5.0kW Hybrid. Odd, I'll try and understand it a bit better. Out of interest, approximately how old is yours?

[zaheerm]

Mine was installed a few weeks ago.

…

On Sun, 20 Feb 2022, 17:59 Dewet Diener, ***@***.***> wrote: Ah, I got it the wrong way around - mine starts with SA but is also definitely a 5.0kW Hybrid. Odd, I'll try and understand it a bit better. Out of interest, approximately how old is yours? — Reply to this email directly, view it on GitHub <#6 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAQ2T35SMJ7HMVJ3ALYCZTU4EFXHANCNFSM5NVTYSKQ> . You are receiving this because you authored the thread.Message ID: ***@***.***>