Skip to content

Conversation

KastB
Copy link

@KastB KastB commented Mar 22, 2025

This pull request adds a number of bug-fixes of wrong Units and registers.
Additionally, we decode the error message as well as the battery type now.
For larger Inverters on the GEN-Input, we can now adjust the CT-Ratio when feeding in too much power to circumvent a Deye bug. For this there are example configurations in the example folder, which work in conjunction to homeassistant.

Copy link
Owner

@luckylinux luckylinux left a comment

Choose a reason for hiding this comment

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

I see you renamed lots of controllers, but where is the new Controller defined ? I don't see the config/modbus.yaml File which I guess would contain 2 Controllers now (with different Refresh Ratews).

- platform: modbus_controller
modbus_controller_id: ${modbus_controller_id}
modbus_controller_id: ${modbus_controller_id_hf}
skip_updates: 2
Copy link
Owner

Choose a reason for hiding this comment

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

Why does this need a high refresh Rate ?

Copy link
Author

Choose a reason for hiding this comment

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

This is outdated, right?

Copy link
Owner

Choose a reason for hiding this comment

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

@KastB: I think this was one of your earlier Commits. Most of the Stuff I feel like you implemented the modbus_controller_id_hf Controller, then a few Commits later you removed it (and switched back to the modbus_controller_id), but originally I expected that you removed it only for a few Signals.

But I think you once again removed it for all of them. I kinda got lost there already.

Basically you tried to change to a high Refresh Rate / dual rate Controller, then I guess you saw it didn't work and preferred to change the Modbus Baud Rate to 115200 instead in Advanced Menu ?


- platform: modbus_controller
modbus_controller_id: ${modbus_controller_id}
modbus_controller_id: ${modbus_controller_id_hf}
Copy link
Owner

Choose a reason for hiding this comment

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

Why does this need a high refresh Rate ?


- platform: modbus_controller
modbus_controller_id: ${modbus_controller_id}
modbus_controller_id: ${modbus_controller_id_hf}
Copy link
Owner

Choose a reason for hiding this comment

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

Why does this need a high refresh Rate ?

filters:
- multiply: 0.1
value_type: U_WORD
value_type: S_WORD
Copy link
Owner

Choose a reason for hiding this comment

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

Did I get the Data Type wrong ? The Reference Guide didn't specify neither U16 nor S16 and I never tested the Generator, so it could very well be. Did you check your Results ?

state_class: "measurement"
accuracy_decimals: 1
value_type: U_WORD
value_type: S_WORD
Copy link
Owner

Choose a reason for hiding this comment

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

Did I get the Data Type wrong ? The Reference Guide didn't specify neither U16 nor S16 and I never tested the Generator, so it could very well be. Did you check your Results ?


- platform: modbus_controller
modbus_controller_id: ${modbus_controller_id}
modbus_controller_id: ${modbus_controller_id_hf}
Copy link
Owner

Choose a reason for hiding this comment

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

Does this really need to have a high Refresh Rate ?


- platform: modbus_controller
modbus_controller_id: ${modbus_controller_id}
modbus_controller_id: ${modbus_controller_id_hf}
Copy link
Owner

Choose a reason for hiding this comment

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

Does this really need to have a high Refresh Rate ?


- platform: modbus_controller
modbus_controller_id: ${modbus_controller_id}
modbus_controller_id: ${modbus_controller_id_hf}
Copy link
Owner

Choose a reason for hiding this comment

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

Does this really need to have a high Refresh Rate ?


- platform: modbus_controller
modbus_controller_id: ${modbus_controller_id}
modbus_controller_id: ${modbus_controller_id_hf}
Copy link
Owner

Choose a reason for hiding this comment

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

Does this really need to have a high Refresh Rate ?


- platform: modbus_controller
modbus_controller_id: ${modbus_controller_id}
modbus_controller_id: ${modbus_controller_id_hf}
Copy link
Owner

Choose a reason for hiding this comment

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

Does this really need to have a high Refresh Rate ?

Copy link
Owner

@luckylinux luckylinux left a comment

Choose a reason for hiding this comment

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

Several Questions, mainly why High Refresh Rate Controller was first implemented, then removed.

// Debug
esphome::ESP_LOGI("main","Modbus: Write - Advanced_BMS_Err_Stop set to %d" , select_value);
ESP_LOGI("main","Modbus: Write - Advanced_BMS_Err_Stop set to %d" , select_value);
Copy link
Owner

Choose a reason for hiding this comment

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

Why this Change (just dropping the esphome Namespace ?) ? Were there some breaking Changes in some ESPHome Updates recently ?


// Debug
esphome::ESP_LOGI("main","Modbus: Write - Previous Register 178 Value (unmodified) = %d", current_value);
ESP_LOGI("main","Modbus: Write - Previous Register 178 Value (unmodified) = %d", current_value);
Copy link
Owner

Choose a reason for hiding this comment

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

Why this Change (just dropping the esphome Namespace ?) ? Were there some breaking Changes in some ESPHome Updates recently ?

"Tianbangda RS485 Modbus": 0x0001
"KOK Protocol": 0x0002
"Keith Protocol": 0x0003
"Topband Protocol": 0x0004
Copy link
Owner

Choose a reason for hiding this comment

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

I get Tuopai Xie over Google Translate instaead of Topband. Unsure if anybody will use it but where did you get your Translation ?

"KOK Protocol": 0x0002
"Keith Protocol": 0x0003
"Topband Protocol": 0x0004
"Pylontech 485 Protocol": 0x0005
Copy link
Owner

Choose a reason for hiding this comment

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

I get Paineng 485 Assistant over Google Translate instead of Pylontech 485 Protocol. Can you explain a bit ? Thanks 😃

"Keith Protocol": 0x0003
"Topband Protocol": 0x0004
"Pylontech 485 Protocol": 0x0005
"Jelais 485 Protocol": 0x0006
Copy link
Owner

Choose a reason for hiding this comment

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

I get Jerry's 485 Assistant over Google Translate instead of Jelais 485 Protocol. Can you explain a bit ? Thanks 😃

state_class: "measurement"
accuracy_decimals: 0
value_type: S_WORD
value_type: S_WORD
Copy link
Owner

Choose a reason for hiding this comment

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

What Change did you make here ? Just removed a NewLine ?

sensor:
- platform: modbus_controller
modbus_controller_id: ${modbus_controller_id_hf}
modbus_controller_id: ${modbus_controller_id}
Copy link
Owner

Choose a reason for hiding this comment

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

Why did you remove the High Refresh Rate Controller for this Signal ?


- platform: modbus_controller
modbus_controller_id: ${modbus_controller_id_hf}
modbus_controller_id: ${modbus_controller_id}
Copy link
Owner

Choose a reason for hiding this comment

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

Why did you remove the High Refresh Rate Controller for this Signal ?


- platform: modbus_controller
modbus_controller_id: ${modbus_controller_id_hf}
modbus_controller_id: ${modbus_controller_id}
Copy link
Owner

Choose a reason for hiding this comment

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

Why did you remove the High Refresh Rate Controller for this Signal ?

state_class: "measurement"
accuracy_decimals: 0
value_type: U_WORD
value_type: U_WORD
Copy link
Owner

Choose a reason for hiding this comment

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

What Change did you implement here ? Just removed a NewLine ?

Copy link
Owner

@luckylinux luckylinux left a comment

Choose a reason for hiding this comment

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

Please clarify where the Menu to Change the MODBUS Baud Rate is exactly Located. I don't remember having seen it. Are you using the same Inverter ?

# Important Notice:
DO NOT USE RS485 ON THE BMS - PORT! This port only has a baudrate of 9600, which is too slow for regular updates.
Use the Modbus port.
Configure it to a baudrate (in the Advanced Menu) of 115200. This allows for updates every second.
Copy link
Owner

Choose a reason for hiding this comment

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

I didn't notice that we could reconfigure the Modbus Baudrate to 115200 ...

In that Case we can definitively increase the refresh Rate and avoid that the ESP32 crashes / gets stuck if refreshing too frequently.

On my Side I have them maybe disappearing from my Network for a Minute or Two each Day, but I thought that it was more related to the DHCP Server renewing IP Address rather than a ESP32 Crash / Reboot (and I still don't think it's that !).

Would that be in Page 2? I cannot see that Setting in the Advanced Menu in the Manual of the Deye Inverter.

image

image

Copy link
Owner

@luckylinux luckylinux left a comment

Choose a reason for hiding this comment

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

Several Comments / Questions.

Copy link
Owner

Choose a reason for hiding this comment

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

This is where I am more concerned.

I originally structured the Repository in:

  • esphome-config-common.yaml
  • esphome-config-master.yaml
  • esphome-config-slave.yaml

Your example/deye.yaml is probably esphome-config-common.yaml merged within esphome-config-master.yaml but somewhat on a complementary Side (it's another Implementation for you who is Grid Connected, you also moved several Settings from separate Config Files into the "main" File).

Not sure how / if we should restructure everything to make it a bit more Consistent, since it's a bit of à La Carte Menu, where everybody can choose the different Components they would like. And obviously no 2 Configurations will ever be the same.

Any Tips ?

bitmask: 0x1

sensor:
# New - To be Tested
Copy link
Owner

Choose a reason for hiding this comment

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

Why did you remove all of these Sensors after you Added them a few Commit earlier ?

@@ -1,24 +1,3 @@
text_sensor:
Copy link
Owner

Choose a reason for hiding this comment

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

Why was this removed ?

return x;
#new / untested
- platform: template
Copy link
Owner

Choose a reason for hiding this comment

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

Looks very interesting 👍 (but I cannot really evaluate the Logic of the Code without diving into it in Detail & troubleshooting with the Deye Inverter).

@luckylinux
Copy link
Owner

@KastB: if you can please reply to my Questions, then I will solve them one by one and proceed to merge.

Thanks for the Help and the Contribution 👍

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