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

H1 Gen2 Inverter support #585

Merged
merged 11 commits into from
Apr 7, 2024
Merged

H1 Gen2 Inverter support #585

merged 11 commits into from
Apr 7, 2024

Conversation

paultweedy77
Copy link
Contributor

First go at H1-*-G2 modbus support based on the register mapping at

https://github.com/TonyM1958/HA-FoxESS-Modbus/blob/main/custom_components/HA-FoxESS-Modbus/modbusH1G2_RS485_LAN.yaml

Things that don't seem to work:

  • Charge periods - can't get the sensors to appear, charge period card doesn't load
  • Inverter fault codes - no mapping for these yet
  • bms_kwh_remaining is 0.0 kWh (but I just had a cube slave added so the BMS might not have worked it out yet?)

https://github.com/TonyM1958/HA-FoxESS-Modbus/blob/main/custom_components/HA-FoxESS-Modbus/modbusH1G2_RS485_LAN.yaml

Things that don't seem to work:
- Charge periods - can't get the sensors to appear, charge period card
  doesn't load
- Inverter fault codes - no mapping for these yet
- bms_kwh_remaining is 0.0 kWh (but I just had a cube slave added so the
  BMS might not have worked it out yet?)
@TonyM1958
Copy link

TonyM1958 commented Apr 6, 2024

Also got nothing for Residual so that's not present on G2 as it stands. Also no registers available for BMS Cell Volts or Temps high / low or BMS cycle count.

Edit: I checked the scans we did for Charge times and values are there but after schedules were enabled, all values went to zero.

@paultweedy77
Copy link
Contributor Author

paultweedy77 commented Apr 6, 2024

Also inverter state doesn't look right. Holding register 39063 returns '4' when the G2 is 'On Grid', which doesn't map to either the H1 or KH lists in ModbusInverterStateSensorDescription.

@TonyM1958
Copy link

    - name: "Inverter Status 1 Code"
      unique_id: foxess_inv1_inverter_status_1_code
      address: 39063
      slave: !secret foxess_inv1_slave
      data_type: uint16 # bitfield, bit0: Standby, bit2: Operation, bit6: Fault (1)
      state_class: measurement
      scan_interval: !secret scan_interval_medium
      # reserved. 39064 Inverter Status 2 Code?
    - name: "Inverter Status 3 Code"
      unique_id: foxess_inv1_inverter_status_3_code
      address: 39065
      slave: !secret foxess_inv1_slave
      data_type: uint32 # bitfield, bit0: On-Grid/Off-grid (0/1)
      state_class: measurement
      scan_interval: !secret scan_interval_medium

@paultweedy77
Copy link
Contributor Author

Hmm.. adding support to mask out bitwise fields is definitely stretching my Python skills, I'm a Perl guy..

We need a new InverterModel. Also order H1_G2 before H1_G1 to avoid
negative lookahead.
@canton7
Copy link
Collaborator

canton7 commented Apr 6, 2024

Hmm.. adding support to mask out bitwise fields is definitely stretching my Python skills, I'm a Perl guy..

Where do you need to do this?

Python uses normal bitwise operations to mask out things: a & ~b etc

@canton7
Copy link
Collaborator

canton7 commented Apr 6, 2024

Thanks for this! I've gone through all of the registers, and they all look in line with @TonyM1958's findings, unless noted

It looks like we might need to have a new sensor for the status code registers. Not sure what we want to do there, whether to show the same thing as the other inverters for consistency (and lose some information in the process), or have 2 different sensors.

I've added some commits to the branch. I've fixed an issue with the inverter profiles, and made a few style tweaks.

@paultweedy77
Copy link
Contributor Author

Yeah there are likely mistakes.. I was hacking around at midnight, keen to have HA working this morning in some form, and I'm definitely not fluent in Python.

Copy link
Contributor Author

@paultweedy77 paultweedy77 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

@canton7
Copy link
Collaborator

canton7 commented Apr 6, 2024

OK, so just questions around ac_power_limit_down, pwr_limit_bat_up, and what we do with the status code.

Does everything else seem to be working OK?

@paultweedy77
Copy link
Contributor Author

I've just noticed in the log:

2024-04-06 15:55:29.947 WARNING (MainThread) [custom_components.foxess_modbus.entities.modbus_entity_mixin] Value (1010: 101.0) for entity 'sensor.fox_h1_inverter_pv1_current' address(es) '[39071]' failed validation against rule (Range : {'_min': 0, '_max': 100})
Register 39071 does indeed read values like:
[39071]: 900

@TonyM1958 has pv*_current scaled by 0.01, so looks like this needs to be different for the G2.

@TonyM1958
Copy link

TonyM1958 commented Apr 6, 2024

The PV Volts, Current and Power are still available at the H1 holding addresses if you want to simplify the configuration by using 31000 - 31007 with the existing scaling. Just a little less precision on the current.

@TonyM1958
Copy link

Probably best to keep things simple.

You can get "Battery Temperature" at 31023 holding as well by the look of it.

Also, current models only have PV1 and PV2 so you don't need to include PV3 and PV4 if you want to clean things up a bit - those are my address place holders in case inverters get more MPPT in the future.

That should help align more of the registers between H1_G1 and H1_G2.

@canton7
Copy link
Collaborator

canton7 commented Apr 6, 2024

Having extra registers or tweaking the scaling a bit isn't really any extra effort -- you can see from the diff that it's only an extra line.

@canton7
Copy link
Collaborator

canton7 commented Apr 6, 2024

Right, fixed those.

Can you also test the Force Charge / Force Discharge work modes please?

@paultweedy77
Copy link
Contributor Author

paultweedy77 commented Apr 6, 2024

Great, I’ll test in a hour once the kids are asleep ..
Won’t be able to test those work modes yet as not on latest G2 firmware. I’ve a ticket in with Fox to fix that, so hopefully next week.

@canton7
Copy link
Collaborator

canton7 commented Apr 6, 2024

Thanks! The force charge/discharge stuff isn't related to the mode scheduler -- we implement it ourselves using the remote control registers, and as far as I know it should work with the firmware you have.

@paultweedy77
Copy link
Contributor Author

No sun, so can't verify pv*_current yet but nothing's newly broken.. PV3 & PV4 entities correctly gone. I'll give the force modes a go now..

Now it's after sunset, have also noticed some wacky out-of-range spiky values coming in on PV1 Power, PV2 Power and PV Power occasionally:

Screenshot 2024-04-06 at 20 53 55

Almost certainly an issue with the inverter firmware.. my old H1 didn't do this.

@canton7
Copy link
Collaborator

canton7 commented Apr 6, 2024

Ah. They're all around 65000, which is suspicious.... Can you use the Read Registers service (Developer Tools - Services) to read one of those registers?

@paultweedy77
Copy link
Contributor Author

paultweedy77 commented Apr 6, 2024

I'll use mbpoll as it's quicker :)

[39279]: 0
[39280]: 0
[39281]: 0
[39282]: 0

Hasn't happened for a while so I'll try to catch it when it does and then also see what 31002 & 31005 say.

@paultweedy77
Copy link
Contributor Author

Force Charge & Force Discharge work modes both worked as expected. 👍

@canton7
Copy link
Collaborator

canton7 commented Apr 6, 2024

Cheers. It does somewhat imply that there's a problem in my code, as those registers are signed and so shouldn't go above 32767, but I can't spot where it is.

@canton7
Copy link
Collaborator

canton7 commented Apr 6, 2024

Aah, pv_power is 2 registers wide on the G2, I forgot that. The fact that it's wrapping at 65535 does rather suggest that it isn't actually 2 registers wide however.

@canton7
Copy link
Collaborator

canton7 commented Apr 6, 2024

I've added an inverter state sensor for the G2. Provided that works, I think the only remaining thing is these weird 65535 readings?

@TonyM1958
Copy link

TonyM1958 commented Apr 6, 2024

PV Power Now is calculated on H1 as the sum of PV1 Power etc. The new sum covers up to 24 PV inputs and is 32-bits for that reason. But, its not the only sensor that does not conform with other 'sources'. I think you could just drop it and work it out as previously?

@canton7
Copy link
Collaborator

canton7 commented Apr 6, 2024

PV Power Now is calculated on H1 as the sum of PV1 Power etc. The new sum covers up to 24 PV inputs and is 32-bits for that reason. But, its not the only sensor that does not conform with other 'sources'. I think you could just drop it and work it out as previously?

I hadn't even noticed that the G2 provided a pv_power_now register. This PR still calculates it by summing pvx_power, as on the H1. That's unrelated to the fact that the 2-register pv1_power / pv2_power registers seem to wrap as if they were single 16-bit registers.

@paultweedy77
Copy link
Contributor Author

paultweedy77 commented Apr 6, 2024

I've added an inverter state sensor for the G2. Provided that works, I think the only remaining thing is these weird 65535 readings?

Inverter State (Fox H1 Inverter): On Grid

Looks good. Will check it changing tomorrow afternoon when I test EPS mode again.
Fantastic work... from zero to fully functional in 24 hours! Only a few useful registers still missing/not working like BMS kWh remaining (but you can fake that with a template sensor so not a huge deal).

Haven't seen any weird PV power readings since 20:50 this evening. Will report if any more are recorded.

@canton7
Copy link
Collaborator

canton7 commented Apr 7, 2024

I'm going to merge this in: the H3-Pro spec just landed, and that's largely the same as the G2. If there are further fixes needed to can get those in separately before the next release.

Thanks for your help!

@canton7 canton7 merged commit 0ca4041 into nathanmarlor:main Apr 7, 2024
3 checks passed
@paultweedy77 paultweedy77 deleted the h1_g2 branch April 7, 2024 13:48
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.

None yet

3 participants