Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add H3 support #167

Merged
merged 18 commits into from
Apr 24, 2023
Merged

Add H3 support #167

merged 18 commits into from
Apr 24, 2023

Conversation

canton7
Copy link
Collaborator

@canton7 canton7 commented Apr 19, 2023

First off, it's probably worth doing this one commit by commit... It's pretty large in parts! Sorry.

This comes in a few stages (see the commit messages for more details)

  1. Move each entity's address from the EntityDescription to the entity itself. The ModbusController then gets the list of addresses to read from the set of entities which have registered themselves on it
  2. Instead of putting one entity address on each EntityDescription, we put a little mapping from inverter model and connection type to address. This means we can ask the EntityDescription whether it supports a particular model and connection type, and it's able to create an entity which has the appropriate address on it. This means we can share entity descriptions across inverter types and connection types
  3. It turns out that all of the register definitions for the H3 are for holding registers. Change from using connection type in point 2, to using register type. We no longer say that e.g. all AUX connections use input registers
  4. Add the H3 registers.

It turns out that whoever wrote the H3 YAML file managed to identify some registers that we didn't, so copy those across for all register types:

 - input_energy_total
 - input_energy_today
 - load_energy_total
 - load_energy_today

(load_energy_total duplicates load_power_total, and it's got the consistent name, so remove load_power_total)

@canton7 canton7 marked this pull request as draft April 19, 2023 16:12
@canton7 canton7 marked this pull request as ready for review April 19, 2023 16:19
@canton7
Copy link
Collaborator Author

canton7 commented Apr 20, 2023

This probably wants double-checking against StealthChesnut/HA-FoxESS-Modbus#95 (comment)

@canton7
Copy link
Collaborator Author

canton7 commented Apr 20, 2023

Argh, looks like the formatter version used by your action is different to the one I have installed... That or it's a configuration difference.

@canton7
Copy link
Collaborator Author

canton7 commented Apr 22, 2023

I also want to disable the EPS sensors by default

…actory

Previously, the EntityFactory knew all of the addressses that the entity
it described would rely on. This was passed to the
InverterModelConnectionTypeProfile, which compiled the list of all
addresses the controller cared about.

Move to a model where the entity itself specifies its addresses. Instead
of the addresses being passed through the
InverterModelConnectionTypeProfile, instead we now record an entity/s
addresses when it registers with the controller (and remove them when it
unregisters).

This doesn't change much right now, but it has two important consequences:
1. An entity's addresses can now be set when it is instantiated, rather
   than being fixed on its description. This lets an entity have different
   addresses depending on the inverter model it's placed on.
2. We now have the option of unregistering an entity when it's disabled, so
   it won't get updates.
The idea here is that we have a single list of entities descriptions. Each
descriptions says the types of inverters and connection types it supports,
and for each combination the address(es) of the register(s) it relys on.

When we come to actually create the entities, we filter to the ones which
support our inverter / connection type combination, and use the address
from that combination.

This caught a bug in the LAN sensors: the inverter/ambient sensors were
swapped for AUX, but not for LAN.
Turns out that the H3 uses the input registers for everything.
Also add the following registers to other types, which were identified in
the YAML file:

 - input_energy_total
 - input_energy_today
 - load_energy_total
 - load_energy_today

load_energy_total duplicates load_power_total, and it's got the consistent
name, so remove load_power_total
@canton7
Copy link
Collaborator Author

canton7 commented Apr 23, 2023

I think this is now pretty much there... I'll merge tomorrow.

@canton7 canton7 merged commit 3842a75 into nathanmarlor:main Apr 24, 2023
@canton7 canton7 deleted the feature/h3 branch April 24, 2023 12:09
@benbeton
Copy link

benbeton commented Apr 26, 2023

thank you for the first beta integration, when adding it to HA i'm receiving the following Info:
image

H3 Inverter - USR-W610

@canton7
Copy link
Collaborator Author

canton7 commented Apr 26, 2023

Thanks, moved to #197. Would you mind joining me there?

canton7 added a commit to canton7/foxess_modbus that referenced this pull request May 30, 2023
This was originally fixed in nathanmarlor#168, but nathanmarlor#167 seems to have accidentally reverted it.

Fixes: nathanmarlor#299
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants