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

Proposal: Rework of code and adding features #110

Draft
wants to merge 176 commits into
base: main
Choose a base branch
from

Conversation

munzili
Copy link
Contributor

@munzili munzili commented May 18, 2022

Hallo!

I'm currently working on refactoring the code in some places and adding also new features. As those are some "breaking changes" (topic changes, code setup process changes) i want to discuses them before creating a "final" pull request. The code is not fully tested yet but compiles and runs - so this is a early sate.

I just want to be sure that those changes are OK or maybe as those are breaking changes this should be released under a new version/tag/branch ... At the moment i did not update the documentation directory - will be done when changes should be ok.

After all i'm trying also to add CAN support to read the Rotex/Altherma CAN Bus based on the pyhspu project. Not sure if it gona work but i will give it a try. I also had ideas to refactore the code setup process in generell. At the end there should not be any code changes needed. Just download a precompiled firmware and config the ESP32 over Wifi/Webserver. Would make it simpler for non-programmers to use it.

So far i made those changes:

  • added cooling modus control. If correctly connected, both heating and cooling can be controlled
  • added ESP32 WROOM dev board
  • fixed state remembering for SG1 and SG2
  • changed setup process. Instead of modifying a file(setup.h and a altherma*.h definition file), witch is registered in git and creates modification messages/conflicts on pull, user should copy a local version witch gets ignored from git and modify it. This way a pull will not create a conflict. This should be done for setup.h and the desired altherma*.h register file
  • extracted EEPROM methods into own header file
  • added CAN lib to be used in future version (Read Rotex/Altherma CAN Bus also)
  • renamed MySerial to a more clear description SerialX10A
  • refactored topic names for callback to have same semantic
  • added header makros to avoid recursive include

@munzili
Copy link
Contributor Author

munzili commented Apr 15, 2023

So, after some time i almosted finished by ideas and features mentioned. My latest commit is a huge one but allmost replaced the whole codebase of the existing ESPAltherma implementation. Following changes have been done

Code Base

  • Refactored code into more cpp and hpp files and classes. This allows for faster compilation when changes are done.
  • Changed default filesystem to LittleFS with min_spiffs.csv as the default partition.
  • Added ESP-IDF as a framework to compile a custom version. ESP32-Arduino is precompiled with an ESP-IDF that doesn't support Rev2 and upwards ESP32 chips, which are needed for 20kbps CAN support for the SJA1000 controller.
  • Added ESP32 board support.
  • Dropped ArduinoOTA support and replaced it with Web OTA.

Definition Files X10A

  • Formatted them to JSON format.
  • Language files from the same model will now be generated from a base file. This reduces maintenance of the definition files.
  • Added presets for models to be selected.

Definition Files CAN-Bus

  • Created a CAN-Bus definition file in JSON format (for now, only HPSU commands have been created).
  • Language files from the same model will now be generated from a base file. This reduces maintenance of the definition files.

Config/Setup Process

  • All configs are now web-based.
  • By default, a standalone WiFi will be generated so the configuration can be changed easily.
  • Configuration can be easily changed at any time without the need to recompile and upload the firmware.
  • Added more config options.
  • Added cooling pin configuration. It is now possible to set a pin/relay for Heating and Cooling mode of the Heatpump.

WebUI

  • Supports Ethernet/WiFi and MQTT configuration options. Allows to scan reachable WiFi networks and select them.
  • Supports X10A parameter selection and tests.
  • Supports CAN-Bus commands selection and tests.
  • Supports firmware update over the web.
  • Supports WebSerial page for debugging and testing.
  • Added automated gzip compression for HTML/CSS/JS files.
  • Split config options into different categories.
  • Added API call to restart the ESP32.
  • Added export and import of configuration.

MQTT

  • Allows customized topics.
  • Dropped MQTT Serial support (replaced with WebSerial)

CAN-Bus

  • Supports CAN-Controller MCP2515, ELM327 and SJA1000.
  • Added CAN-Sniffing mode so commands of the Heatpump can be easily analyzed and new definition files created.
  • Added readonly mode to ensure Heatpump CAN-Bus not getting disturbed.
  • Added different CAN Poll modes (disabled, passive, and auto).
  • Custom commands can be added via the web.

X10A

  • Allows selection of presets/filtered list of parameters.
  • All available X10A commands in a parameters file or from the select list can be prefetched via WebUI and verified. These have correct values.

Todo

  • Merge master-branch changes since May 2022.
  • Update readme to new functions and features.
  • Implement CAN-Bus data preview.
  • Version the firmware and display it on the WebUI.
  • Refactor the code base more to be more SOLID.
  • CAN-Controller ELM327 is untested and needs to be tested.
  • Homeassistant support not implemented yet. Maybe more smarthome options should be supported (For me Loxone is in the todo list).
  • Create a CI script to generate an automated release build with precompiled binaries and definition files. This allows for simple updates by downloading and uploading it on ESPAltherma.

The above Todo list is still open, and the code is currently in beta status. I have been running my new code on an ESP32 (specifically the ESP32-WROOM-32E) for some time now, and it has been functioning well with a Rotex HPSU compact that has X10A and CAN (SJA1000) support. I would appreciate any feedback you may have. Additionally, I am wondering how such a significant change could be merged into the master branch. Perhaps it would be better to maintain it as a separate branch or version.

EspAltherma Config

merge main changes
@DieterTHeck
Copy link

First feedback: Wow.
What is the best way to beta-test it? Compile your :rework files?

@DieterTHeck
Copy link

regarding X10a definitons. Clever move to decouple translation and actual addresses.
I wonder if it would not be even easier to maintain if we had "vectors" with English, German, French, ... names for the same thing in one row but differnt columns. Better overview, less files. For English take entry[,0], for German entry[,1] etc.

Further feedback once I had time to have an actual look.

@munzili
Copy link
Contributor Author

munzili commented Apr 19, 2023

To test/tun it:

  • Clone repo with platformio
  • clone submodules with: git submodule update --init --recursive
  • compile and upload it

To generate CAN and X10A definition files (will be created in build directory):

  • run script "scripts/build_can_commands.py"
  • run script "scripts/build_x10a_commands.py"

@raomin
Copy link
Owner

raomin commented Apr 19, 2023

Hey @munzili, quite some impressive work in here indeed. Thanks!

I'll need some time to review it and would suggest to split it in smaller PR easier to discuss and merge.

regarding X10a definitons. Clever move to decouple translation and actual addresses. I wonder if it would not be even easier to maintain if we had "vectors" with English, German, French, ... names for the same thing in one row but differnt columns. Better overview, less files. For English take entry[,0], for German entry[,1] etc.

I also would need to have a look but that would mean to load significantly the flash with all the translations. That's significant.

@munzili
Copy link
Contributor Author

munzili commented Apr 20, 2023

Splitting into smaller pieces for PR could be quite difficult.

The main change is to use web UI to configure and control the system instead of compiling the config into it.

This means setup.h and the definitions can't be .h files anymore. Therefore, a bigger code rewrite is needed to support it. Those changes are just coupled together and can't be created in a separate PR.

What can be split into an own PR is CAN Support and all changes to support it, like including own ESP-IDF and therefore dropping ESP8266 support. But this feature requires a WebUI, so the PR would depend on the first (WebUI) one.

Of course, there are new features added with the web UI which can be created into an own PR, but those would not make sense in my opinion as those changes belong together. These are changes like:

  • Export/import config over the web
  • Firmware update over the web (back to Arduino OTA other way)
  • Webserial support (back to log text over MQTT again)
  • Live X10A preview of selected commands

@munzili
Copy link
Contributor Author

munzili commented Apr 20, 2023

fixed merge (created too much history comments)

@munzili munzili reopened this Apr 20, 2023
@maromme
Copy link

maromme commented Apr 20, 2023

wow, that looks like a bunch of work... I've to admit, I can't get it running. The description above doesn't help me much. Is it possible to download a full "version" I just have to compile and upload?

@bzq-pl
Copy link

bzq-pl commented Apr 20, 2023

I must admit I couldn't compile :-( I have uploaded recent version of code and during linking I have such error:


Linking .pio/build/esp32/firmware.elf
*/.platformio/packages/toolchain-xtensa-esp32/bin/../lib/gcc/xtensa-esp32-elf/8.4.0/../../../../xtensa-esp32-elf/bin/ld: .pio/build/esp32/src/main.o: in function setup()': ###/src/main.cpp:65: undefined reference to fs::LittleFSFS::begin(bool, char const, unsigned char, char const)'
***/.platformio/packages/toolchain-xtensa-esp32/bin/../lib/gcc/xtensa-esp32-elf/8.4.0/../../../../xtensa-esp32-elf/bin/ld: .pio/build/esp32/src/Config/config.o:(.literal._Z10readConfigv+0x4): undefined reference to LittleFS' ***/.platformio/packages/toolchain-xtensa-esp32/bin/../lib/gcc/xtensa-esp32-elf/8.4.0/../../../../xtensa-esp32-elf/bin/ld: .pio/build/esp32/src/WebUI/webui.o:(.literal._Z19onWebSerialCallbackPKhj+0x30): undefined reference to fs::LittleFSFS::usedBytes()'
***/.platformio/packages/toolchain-xtensa-esp32/bin/../lib/gcc/xtensa-esp32-elf/8.4.0/../../../../xtensa-esp32-elf/bin/ld: .pio/build/esp32/src/WebUI/webui.o:(.literal._Z19onWebSerialCallbackPKhj+0x34): undefined reference to fs::LittleFSFS::totalBytes()' ***/.platformio/packages/toolchain-xtensa-esp32/bin/../lib/gcc/xtensa-esp32-elf/8.4.0/../../../../xtensa-esp32-elf/bin/ld: .pio/build/esp32/src/WebUI/webui.o:(.literal._Z15formatDefaultFSv+0x10): undefined reference to fs::LittleFSFS::end()'
*/.platformio/packages/toolchain-xtensa-esp32/bin/../lib/gcc/xtensa-esp32-elf/8.4.0/../../../../xtensa-esp32-elf/bin/ld: .pio/build/esp32/src/WebUI/webui.o:(.literal._Z15formatDefaultFSv+0x14): undefined reference to fs::LittleFSFS::format()' ***/.platformio/packages/toolchain-xtensa-esp32/bin/../lib/gcc/xtensa-esp32-elf/8.4.0/../../../../xtensa-esp32-elf/bin/ld: .pio/build/esp32/src/WebUI/webui.o:(.literal._Z15formatDefaultFSv+0x18): undefined reference to fs::LittleFSFS::begin(bool, char const, unsigned char, char const)'
***/.platformio/packages/toolchain-xtensa-esp32/bin/../lib/gcc/xtensa-esp32-elf/8.4.0/../../../../xtensa-esp32-elf/bin/ld: .pio/build/esp32/src/WebUI/webui.o: in function onWebSerialCallback(unsigned char const*, unsigned int)': ###/src/WebUI/webui.cpp:1005: undefined reference to fs::LittleFSFS::usedBytes()'
***/.platformio/packages/toolchain-xtensa-esp32/bin/../lib/gcc/xtensa-esp32-elf/8.4.0/../../../../xtensa-esp32-elf/bin/ld: ###/src/WebUI/webui.cpp:1005: undefined reference to fs::LittleFSFS::totalBytes()' ***/.platformio/packages/toolchain-xtensa-esp32/bin/../lib/gcc/xtensa-esp32-elf/8.4.0/../../../../xtensa-esp32-elf/bin/ld: .pio/build/esp32/src/WebUI/webui.o: in function formatDefaultFS()':
###/src/WebUI/webui.cpp:12: undefined reference to fs::LittleFSFS::end()' ***/.platformio/packages/toolchain-xtensa-esp32/bin/../lib/gcc/xtensa-esp32-elf/8.4.0/../../../../xtensa-esp32-elf/bin/ld: ###/src/WebUI/webui.cpp:13: undefined reference to fs::LittleFSFS::format()'
*/.platformio/packages/toolchain-xtensa-esp32/bin/../lib/gcc/xtensa-esp32-elf/8.4.0/../../../../xtensa-esp32-elf/bin/ld: ###/src/WebUI/webui.cpp:18: undefined reference to `fs::LittleFSFS::begin(bool, char const, unsigned char, char const)'
collect2: error: ld returned 1 exit status
*** [.pio/build/esp32/firmware.elf] Error 1


Looks like some problem with LittleFSFS library - I have checked and I have this library installed ...

@munzili
Copy link
Contributor Author

munzili commented Apr 20, 2023

@bzq-pl
Please make sure:

  • Use "Clean All" inside platformio to get a clean build directory
  • run command "git submodule update --init --recursive" in terminal before compile. This will download the LittleFS submodule/ESP-IDF component.
  • run "Build" command inside platformio

@bzq-pl
Copy link

bzq-pl commented Apr 20, 2023

@munzili
thanks!!! - that works! :-)

@munzili
Copy link
Contributor Author

munzili commented Apr 20, 2023

@maromme
where exactly do you have problems? Do you have problems compile it? What board and environment do you use?

@maromme
Copy link

maromme commented Apr 20, 2023

@munzili well platformio and using github for more than downloading projects is new to me, there I would need more detailed instructions. I did all my previous projects (a lot on esp32) in arduino IDE.
guess I'm lost track of the fancy new possibilities in programming. I started quite a while ago with html without wysiwyg editors and php without big libraries...

@munzili
Copy link
Contributor Author

munzili commented Apr 21, 2023

@maromme
i made a quick and dort readme file update in my rework branch. I updated the sedition Getting started to descript the build process and config options. This is just a fast change, are more clear update inside readme will be done later.

Please take a look if it helps building it.

@maromme
Copy link

maromme commented Apr 23, 2023

well, that's getting a bit off topic now: I can't exec "git submodule update --init --recursive". Guess I need some kind extension to interact with github? Besides that, thank's for the explanations. Think I'll be fine with that then

@munzili
Copy link
Contributor Author

munzili commented Apr 24, 2023

@maromme
please check if you have git installed (https://git-scm.com/downloads). Git is needed so the command can be runned. also updated it in the readme files

@munzili munzili reopened this Apr 24, 2023
@ap20132022
Copy link

@munzili: thank you very much for this update. It's a major great update, compliment.
After some beginning's problem, I installed this pull request to my device. It's looks like it works.
Now my questions:

  1. firstly I couldn't find the SSID to connect to the ESP. I think and hope that at first ESP allows me to connect (e.g. over Smartphone) to get access to ESP to make some changes (like Tasmota). Is it true? After a while, I change the setup.h file to let ESP connect to my router. It works.
  2. I have now a valid IP for my ESP. How can I get access to the webUI? If I type the IP on my Chrome, the website can't be shown. Do I need anything more? Sketch? https://lastminuteengineers.com/esp32-ota-updates-arduino-ide/?utm_content=cmp-true
  3. which CAN-Device is recommended? I have already buy an USB-CAN. So it wouldn't work with this ESP?
    Thank you very much for your help.

@maromme
Copy link

maromme commented Apr 25, 2023

@munzili : thank's for your support! I can compile, but get an error. See attached file. I run the git submodule... etc before compiling
log (2).txt

edit: maybe it's my "thing": the nice thing about the main branch of @raomin is the simplicity of installing and setup (if you find the right definition file). that opens up the project for users with minimal programming skills. Right now, I've got the feeling that the installing process (compiling and upload) is a bit "trappy". There are many more things that could go wrong. To keep the simplicity, I guess a project like that should be more "closed": no external dependencies etc. If I think of my #273 I don't like the external library as well.
Another approach could be pre compiled files that are uploaded with a flash tool for certain boards?

@KindeG
Copy link

KindeG commented Jun 21, 2024

Hello @munzili,
let me ask how's the project is going? :)

@munzili
Copy link
Contributor Author

munzili commented Jun 23, 2024

I already have a stable working version - I keep my ESPAltherma up to date with my latest changes, and it is running stably without problems. I also got rid of the git submodule error and created the first unit tests. The CAN reading and writing is working fine, but the protocol doesn't work yet. Either my heat pump (Rotex HPSU 516) is not compatible with the code from https://github.com/ahermann86/fhemHPSU and https://github.com/zanac/pyHPSU, as I tried to recreate their code for ESPAltherma, or there is still a problem in the code. Another problem is that ESP8266 support is still not working. The code can be compiled and installed correctly, but the chip has too little RAM to handle the web server, web updates, X10A, and CAN scans.

I will not mark this PR as stable until ESP8266 support works because of backward compatibility, unless raomin maybe uses the code as a 2.0 codebase or so to keep backward compatibility.

@munzili
Copy link
Contributor Author

munzili commented Jun 23, 2024

my main working branch ist the feature-webui branch, the rework branch is for this PR

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

9 participants