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

Improvements: Webserver / integration simplification #617

Merged
merged 6 commits into from
Nov 15, 2024

Conversation

dalathegreat
Copy link
Owner

@dalathegreat dalathegreat commented Nov 14, 2024

What

This PR implements simplified webserver & battery/inverter integrations

Why

To simplify further development and make scaling the software up easier

How

  • Instead of large #ifdef blocks inside Webserver.cpp to signal what battery/inverter is used, we now write a text string to the datalayer on startup. The webpage displays both battery_protocol and inverter_protocol. This simplifies development, no longer needed to add inverter/battery specific text outside of the battery's .cpp file.
  • init_inverter() now calls setup_inverter() in each inverter protocol. Code required specifically for inverter protocol is now inside that inverter protocols .cpp file. This simplifies development, with less risk of forgetting inv specific stuff
  • All #ifdef intervalUpdateValues removed, since code now updates at 1s rate instead of 5s
  • Startup DEBUG_VIA_USB texts removed
  • BONUS: Some inverter/batteries were missing from the Github workflow. Added them to build system

@nagyrobi
Copy link

Although it would probably require a serious amount of work at first, maybe it's worth considering implementing all these as ESPHome components.

ESPHome is a very well designed, extremely modular environment. It natively supports CAN and Modbus as frameworks, adding discrete devices on top of these is pretty straightforward. Has webserver support, MQTT fully customizable.
Easier to maintain on long term.

@dalathegreat
Copy link
Owner Author

Although it would probably require a serious amount of work at first, maybe it's worth considering implementing all these as ESPHome components.

ESPHome is a very well designed, extremely modular environment. It natively supports CAN and Modbus as frameworks, adding discrete devices on top of these is pretty straightforward. Has webserver support, MQTT fully customizable. Easier to maintain on long term.

Feel free to fork it there! I personally dont use MQTT or Webserver, I focus mainly on getting a stable and reliable way to re-use the batteries.

@clowrey
Copy link
Contributor

clowrey commented Nov 14, 2024

Although it would probably require a serious amount of work at first, maybe it's worth considering implementing all these as ESPHome components.

ESPHome is a very well designed, extremely modular environment. It natively supports CAN and Modbus as frameworks, adding discrete devices on top of these is pretty straightforward. Has webserver support, MQTT fully customizable. Easier to maintain on long term.

I want to do a small test with ESPhome soon - I already have some of the Tesla M3 battery protocol decoded via ESPhome, mainly just need to emulate the BYD HV protocol - any interest in helping with that @nagyrobi ? I don't think it will be particularly hard and could be done all in YAML, or via an external component. We probably should start another thread on the discord or somewhere else to discuss this though.

@clowrey
Copy link
Contributor

clowrey commented Nov 14, 2024

For this commit I fully support removing as many #ifdef blocks as possible from the code base :).

Somehow getting the dual batteries to not require doubling the Tesla.cpp file would be a huge improvement too eventually. Probably by making everything classes etc.

@dalathegreat
Copy link
Owner Author

Somehow getting the dual batteries to not require doubling the Tesla.cpp file would be a huge improvement too eventually. Probably by making everything classes etc.

This is on my TODO list, I really want to refactor the battery handling as a class (which would get rid of duplicate code). But a bit too much for this PR :)

@nagyrobi
Copy link

any interest in helping with that @nagyrobi

Sure!

Copy link
Collaborator

@amarofarinha amarofarinha left a comment

Choose a reason for hiding this comment

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

Very nice! LGTM.

@dalathegreat dalathegreat merged commit f2ad0c5 into main Nov 15, 2024
78 checks passed
@dalathegreat dalathegreat deleted the improvement/webserver-string-simplification branch November 15, 2024 20:10
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.

4 participants