hoggyhoggy / givenergy-modbus-async

A python library to access GivEnergy inverters via Modbus TCP on a local network, with no dependency on the GivEnergy Cloud.
Other
1 stars 2 forks source link

another round of changes for review #33

Closed divenal closed 1 month ago

divenal commented 1 month ago

Changes over to use relative imports

removes Arrow dependence Some register updates Adds some docs and comments.

One interesting feature is generation of Inverter and Battery classdoc on-the-fly from the register list.

Doesn't include the proposed change to avoid trivial setters from commands.py since I might have to revisit that.

Just noticed we have duplicate entries for "charge_slot_2" (regs 31/32 and 243/244).

hoggyhoggy commented 1 month ago

"charge_slot_2" (regs 31/32 and 243/244). - That's a Gen 1 thing - We don't have the registers up at 243+ so Gen 1 uses 31/32 -however the charge slot 2 for Gen1 doesn't actually work anyway.... (you change it and it changes itself back to 00:00 - 00:04 for some obscure reason) so moot point anyway!
Gen 1 stops at HR200 (if you exclude that block of 4000 ones)

divenal commented 1 month ago

Gen 1 stops at HR200 (if you exclude that block of 4000 ones)

But you can still ask for 60 registers starting at 180? Does it just pad beyond 200 with zeros? Contrast with latest britkat change which suggests we must not ask ems for more than 36 registers at block 2040. (I'm surprised they wouldn't just pad it out to 60.)

britkat1980 commented 1 month ago

Looks good. I'm not very clued up on how I can track this PR into my code to test how it works with my wip code... I want to test the dyanmic register creation (not using pydantic) because I'm seeing whole read cycles thrown out because of one bad value... Any help gratefully recieved!

divenal commented 1 month ago

Looks good. I'm not very clued up on how I can track this PR into my code to test how it works with my wip code... I want to test the dyanmic register creation (not using pydantic) because I'm seeing whole read cycles thrown out because of one bad value... Any help gratefully recieved!

Your renaming of the tree does make things rather awkward. I'd suggest first thing to do is to adopt the relative import change - that way, things become largely independent of the name of the directory, and you can have it named givenergy_modbus in your wip tree, but still call it 'givenergy_modbus_async' when you drop it into your giv_tcp tree.

git is very powerful - once you understand how it "thinks", it becomes relatively easy to control. Probably easier with git locally than with github. You can set up your local tree to be able to take pull changes from anywhere. eg I have

$ git remote -v
britkat https://github.com/britkat1980/givenergy-modbus-async.git (fetch)
britkat https://github.com/britkat1980/givenergy-modbus-async.git (push)
hoggy   https://github.com/hoggyhoggy/givenergy-modbus-async (fetch)
hoggy   https://github.com/hoggyhoggy/givenergy-modbus-async (push)
origin  git@github.com:divenal/givenergy-modbus-async.git (fetch)
origin  git@github.com:divenal/givenergy-modbus-async.git (push)

So I can bring in changes from any of these (with git fetch xxx) and they all coexist within git. I use a graphical tool 'gitk' which comes with git to visualise the various branches, but I'm sure there are fancier ones.

Once they're all in one git tree, you can use 'cherry-picking' to copy individual commits - I try to keep each commit focussed on a single task, so it should be possible to try this bit without having to take everything else. The cruder way to do things is to create patches - just ask git to produce a patch file, which is just context diffs, and then you can use the 'patch' utility (or something graphical, if you prefer) to try to apply those diffs to your tree. As long as the surroundings haven't changed too much, this can be relatively straightforward. And you should be able to do it this way despite the big rename.

There are actually several copies of the pydantic removal change in different parts of my tree - one of the others might well be closer to your dev code since I was starting from a different place: f5444047511cc0e1635320638e9d572a9b10b0de ac3bb638b45a4b13d7a2ce13f58199608a10e626 (moved around with git rebasing and cherry-picking, but I held onto the originals)

divenal commented 1 month ago

Actually, I think I already have a change which applies the relative imports to your dev tree (so that I could rename it back to cherrry-pick changes) d9447e7fd3c02672e8611b72437a1b4634c744bf

hoggyhoggy commented 1 month ago

Merged as per PR 33