Skip to content

Conversation

@Sopsy
Copy link

@Sopsy Sopsy commented Dec 17, 2025

This is quite a big rework of the whole Deye 3P battery definition.

Secondary batteries are not tested, and also BMS 2 batteries are still not there because it's already extremely long.
I feel like 20 batteries are excessive, but that's how many there were originally, so I kept them. I could not find any information about how many batteries they support.

Old definitions are kept with a modifier.

There are also some things I don't necessarily like, for example how the battery entities are all named sensor.battery_name_battery_1_xxx - the battery_1 feels redundant. I was not able to change them without touching the entity creation code, so I left it as is.

Additionally the Battery Life Cycle counters are off by one decimal point, counting 10 every time the battery is fully charged from emtpy, but they are defined deeper in the code and might be correct for other devices. However, the BMS also calculates it so the entity is totally unnecessary.

The values of all modified entities are confirmed to match with Solarman Cloud.

To note that I created this branch only after the other pull request, so this contains those changes as well.

Opening as a draft because I am unable to test the old entities and because some functions seem to assume there is only one BMS/cluster (like the Life Cycles entities).

Fixes #773 and maybe #896 as well (added all missing battery entities that are available).

Feedback wanted:

  1. Do we really want to keep 20 battery definitions? I would rather do it in the code with 1 definition instead of repeating the definitions. This applies for both the old LV and the new HV definitions.
  2. For BMS2 there can be even more batteries. I would like to add that, but a total of 40+ battery definitions is definitely not a good idea.
  3. Why Battery 1 Alert and Battery 1 Fault always goes under the inverter and not under the battery? They should be under the battery, and the inverter should only have something like a sum sensor saying if any one of the batteries is alerting.
  4. Why suggested_display_precision: 0 does not work? The inverter does not count decimals for some values and displaying them is unnecessary.
  5. There are a lot of entities. Disabling the more advanced ones by default should be a good idea. What do you think, and which?

@davidrapan davidrapan changed the title Fix invalid battery entities for HV, add missing BMS and battery entities | Deye 3P Fix invalid battery entities for HV, add missing BMS and battery entities ❘ Deye 3P Dec 18, 2025
@davidrapan davidrapan force-pushed the main branch 3 times, most recently from 741d5f4 to 36abfef Compare December 22, 2025 22:15
@davidrapan
Copy link
Owner

davidrapan commented Dec 25, 2025

Do we really want to keep 20 battery definitions? I would rather do it in the code with 1 definition instead of repeating the definitions. This applies for both the old LV and the new HV definitions.

I agree, current implementation is temporary, but it has to wait until we redo the "profile database". That is planned for sometime in the future. :)

For BMS2 there can be even more batteries. I would like to add that, but a total of 40+ battery definitions is definitely not a good idea.

Well, I'm not sure if it should be there by default... there is a reason why they are currently just attributes as for most users it's irrelevant, but adds unnecessary traffic and "bloats" it.

Why Battery 1 Alert and Battery 1 Fault always goes under the inverter and not under the battery? They should be under the battery, and the inverter should only have something like a sum sensor saying if any one of the batteries is alerting.

Because registers 10000+ are only for Deye batteries, and Alert + Fault are exposed always.

Why suggested_display_precision: 0 does not work? The inverter does not count decimals for some values and displaying them is unnecessary.

Maybe a bug? :)

There are a lot of entities. Disabling the more advanced ones by default should be a good idea. What do you think, and which?

Yeah, that's the question. :) IMO, since most users do not have batteries from Deye, the additions in this PR are irrelevant for them.

+ I think we should separate changes to BMS and 10000+ registers.

++ Do you think you could add a new unique_id property to newly added entities that would closely match their name from the documentation?

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.

Deye BOS-G: All values ​​are incorrect

2 participants