Repository navigation
Modbus TCP support - #996
ReinhardGruber wants to merge 29 commits into
Conversation
…better clarity and functionality
…egister mapping and binary files
- Introduced S0 input handling with new data structures and functions. - Updated Modbus register mapping to include S0 inputs. - Enhanced README and documentation to reflect S0 configuration and usage. - Modified existing code to integrate S0 readings into the Modbus server. - Added regression tests for S0 inputs to ensure functionality.
IgorYbema
left a comment
There was a problem hiding this comment.
Thanks a lot for contributing this back! Modbus TCP support is welcome in principle, and the register map design (fixed expandable blocks, version register, float word pairs, host-side tests) is well thought out. That said, there are a few things that need to change before we can merge it into the main project.
CI build fails
The build stops at "Install dependencies":
Error installing eModbus: Library 'eModbus@latest' not found
eModbus is not in the Arduino library registry (it's only in the PlatformIO registry), so arduino-cli lib install eModbus can't find it. Also, "AsyncTCP" in the Arduino registry resolves to the old dvarrel 1.1.4 fork; the maintained ESP32Async version is registered as "Async TCP". Something like this should work:
- name: Install dependencies
env:
ARDUINO_LIBRARY_ENABLE_UNSAFE_INSTALL: "true"
run: |
arduino-cli lib install ringbuffer pubsubclient arduinojson dallastemperature onewire "Adafruit NeoPixel" "Async TCP"
arduino-cli lib install --git-url https://github.com/eModbus/eModbus.git#v1.7.5stableThe .devcontainer, LIBSUSED.md and AGENTS.md build instructions need the same two libraries. Please make sure both the ESP8266 and ESP32 builds pass with arduino-cli (the scripts/build_*.sh commands), not only PlatformIO.
Blockers
1. Modbus callbacks run in the AsyncTCP task, not in loop(). From there:
FC_06callssend_heatpump_command(), which callslog_message(), which callsmqtt_client.publish()andwebsocket_write_all(). Neither PubSubClient nor the webserver is thread-safe, so this races with the main loop and can cause random crashes or a corrupted MQTT connection.- The optional-PCB setters change the shared
optionalPCBQuery[]without any locking, while the serial task and the MQTT and web command paths also use it. FC_03decodesactData/actDataExtrawhilereadSerial()may be overwriting them, so a read can return a mix of old and new bytes.logNonNumericTopicValue()also callslog_message()from the async task.
Please hand writes over to the main loop (for example through a queue that loop() drains and then calls send_heatpump_command from there) and log via logQueue. For reads, use a consistent snapshot, for example protected by a mutex.
2. Modbus is always on, with no authentication. Port 502 opens on every ESP32 boot, and anyone on the LAN can write any command, including SetReset (22000) and both relays. The README in this PR says "keep write access disabled until your logic is proven", but no such option exists. Please add Settings options: Enable Modbus TCP (default off) and Allow Modbus writes (default off).
3. Fork-specific changes need to be removed. This PR needs to contain only the feature:
version.hhardcoded to4.2.2-ModbusTCP- Page title and topbar text ("HeishaMon ModBus TCP") and the accent color change (
#3a7bd5→#368DF4) - The "Modbus-Enabled Fork" section at the top of
README.md(a short Modbus section further down, linking toModbus-Register-Mapping.md, is fine) - In
main.yml: the-ModbusTCPversion suffixes and the renamed binaries (HeishaMon_ESP32-ModbusTCP-…). The renamed binaries would also break the MD5 lookup in the firmware upload page, which splits the filename on-. - The committed binaries under
binaries/model-type-large/(v4.04ALPHAandv4.2.2-ModbusTCP) platformio.ini,scripts/platformio_export.pyandscripts/export_firmware.py, plus the change tobuild_*.shthat copies every local build intobinaries/(this dirties the working tree for every developer)
Should fix
commands.h: please don't add anidfield tocmdStruct/commands[]. Keep the Modbus IDs inModbusRegisterMap.has a name→ID table, the same way you already do forOPTIONAL_COMMANDS, so the core command table and anything that parses it (like the rules harness regex you had to change) stay untouched. Astatic_assertor a host test for duplicate IDs would also help.- Writes are int16 ×1: optional-PCB temperatures such as
SetPoolTempandSetZ1RoomTempcan't receive values like 21.5. Reads use ×100 for temperatures, so writes should probably be symmetric for temperature commands. - Optional-PCB writes when the optional PCB is disabled:
FC_06reports success, butsend_heatpump_commandsilently ignores the command. It should return an exception instead (e.g.ILLEGAL_DATA_ADDRESSorSERVER_DEVICE_FAILURE). - Extra-block registers: when
extraDataBlockAvailableis false, these return values decoded from an empty buffer. It's better to return an exception. - Sidebar navigation: moving it from per-page JS into
webBodyStartis a nice cleanup, but it's unrelated to Modbus and changes pages that previously had no nav. Please submit it as a separate PR. - Rules tests step in CI: reasonable, but also separate from this feature.
Minor
- File name casing:
HeishaModbusServer.hvsHeishaModBusServer.cpp. Please pick one. - The comment in the header is in German; please translate it to English.
- Stray blank lines added in
HeishaMon.ino(e.g. in the OpenTherm setup block).
To sum up: a PR with just the Modbus server, the minimal hooks in HeishaMon.ino/webfunctions/gpio/s0, opt-in settings, and main-loop command handling would be much easier to review and merge. Thanks again for the work on this!
Modbus TCP server (port 502) on the large board: read heat pump, optional PCB and S0 values (FC03), send commands (FC06), switch relays (FC05). - Off by default: "Enable Modbus TCP server" and "Allow Modbus writes" in a new Modbus TCP settings panel - AsyncTCP callbacks only read a data snapshot and queue writes; loop() executes them - Modbus command IDs in ModbusRegisterMap.h, commands.h unchanged - Optional PCB temperatures are written x100; unavailable extra/optional registers return ILLEGAL_DATA_ADDRESS - Register page at /modbus, docs, Loxone template, host tests - CI/devcontainer: install Async TCP (ESP32Async) and eModbus from git - Keep platformio.ini for pioarduino builds Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
- Add FC16 float32 writes for all commands (unscaled real value) - Add FC01 read coils and relay state registers for relay readback - Temperature commands are x100 in int16 registers, whole degrees only for heat pump commands - Sort address space by type: int16 measurements, int16 commands, float32 measurements, float32 commands, coils, device information - Register page: single list sorted by address, no command addresses in the description - Bump register map to v3, update docs, Loxone template and tests
|
Thanks for the rework, this addresses almost everything from the previous review. The snapshot/write-queue approach and the opt-in settings look good. A few remaining points:
|
function code and name the entries that differ; keep only the plain g++ path (drop MSVC/emscripten handling not used by CI). - gpio.cpp: remove stray double blank line.
|
Thanks for the review! I've pushed an update:
Once this PR is merged, I have a few follow-ups planned as separate PRs:
Let me know if you'd like any of these discussed in an issue first. |
- Add SetSterilizationTemp (5047, x100) and SetSterilizationMaxTime (5048) from upstream to the Modbus command map and docs. - Loxone template: int16 commands use FC06 at 5000+, Z1 setpoints FC16 float32 at 20008/20010, ErrorInformation back on int16 register 44 (the float register reads 0 for error text). - run_tests.py: expect ErrorInformation at 44. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
I am wondering if I would allow the modbus functionality for the ESP8266 variant. Did you test it on that also? It does have much memory available and the speed isn't that great either. Yes ok, keep the platform.io.
Do make seperate PR's for each of this changes. |
|
Thanks for the quick reply.
Unfortunately I don't have an ESP8266, so I couldn't test it there. As a first step I'd enable Modbus for the ESP32 only.
I'll open separate PRs for each change.
Is there already a timeline for which version Modbus will be released in?
Vielen Dank
Reinhard Gruber
…________________________________
Von: IgorYbema ***@***.***>
Gesendet: Donnerstag, 01. Oktober 2026 07:40
An: heishamon/HeishaMon ***@***.***>
Cc: Gruber Reinhard ***@***.***>; Author ***@***.***>
Betreff: Re: [heishamon/HeishaMon] Modbus TCP support (PR #996)
[https://avatars.githubusercontent.com/u/20365971?s=20&v=4]IgorYbema left a comment (heishamon/HeishaMon#996)<#996 (comment)>
I am wondering if I would allow the modbus functionality for the ESP8266 variant. Did you test it on that also? It does have much memory available and the speed isn't that great either.
Yes ok, keep the platform.io.
* Setting commands has to be explicitly allowed in the config. -> Then it must clearly be what the difference is between listen only mode. Think about that please.
* Wi-Fi: WPA Enterprise support. -> Yes, but probably only for the ESP32?
* Hostname shown in the web UI header. With several heat pumps under remote maintenance, telling them apart by IP alone is error-prone. -> Yes good thinking
* Setting values directly in the web UI. Going through the URL is often too cumbersome. -> Original design was to not do anything on the device itself but let's do this indeed.
* Only send a value to the heat pump if it differs from the current one, whether it arrives via Modbus, MQTT, URL or elsewhere. This reduces EEPROM write cycles on the heat pump. -> Ok for this also
Do make seperate PR's for each of this changes.
—
Reply to this email directly, view it on GitHub<#996?email_source=notifications&email_token=AAX5JPM2XXSE5FAGJJKR5BT5RXU5NA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKOJSGU2DOMBYG4ZKM4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2KYZTPN52GK4S7MNWGSY3L#issuecomment-5925470872>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AAX5JPKBU3PGNVPITIPAZQT5RXU5NAVCNFSNUABFKJSXA33TNF2G64TZHMYTOOBYGMZTMMRVHNEXG43VMU5TKNRTGQ3TENBVGQ32C5QC>.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS<https://github.com/notifications/mobile/ios/AAX5JPJJVEQZBDUTX7LTK735RXU5NA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKOJSGU2DOMBYG4ZKM4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2KUZTPN52GK4S7NFXXG> and Android<https://github.com/notifications/mobile/android/AAX5JPMWLOT47KJVXRAHGXL5RXU5NA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKOJSGU2DOMBYG4ZKM4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2K4ZTPN52GK4S7MFXGI4TPNFSA>. Download it today!
You are receiving this because you authored the thread.Message ID: ***@***.***>
|
|
I very much hope that the Modbus functionality, in particular, will be added to the project. This would make it easy to integrate HeishaMon into the existing building automation system. Almost all control systems support Modbus, but very few support MQTT. The Modbus functionality would really enhance the project. |
|
I agree to enable it for the ESP32 only at first. Can you add the #ifdef lines so the modbus code isn't added to the esp8266 tree? edit: i just noticed that is already the current situation |
|
[cid:26e21b99-604e-4e76-a661-cc9a1e7cf86c]
________________________________
Von: IgorYbema ***@***.***>
Gesendet: Donnerstag, 1. Oktober 2026 08:32
An: heishamon/HeishaMon ***@***.***>
Cc: Gruber Reinhard ***@***.***>; Author ***@***.***>
Betreff: Re: [heishamon/HeishaMon] Modbus TCP support (PR #996)
@IgorYbema commented on this pull request.
________________________________
In HeishaMon/HeishaMon.ino<#996 (comment)>:
@@ -45,6 +45,7 @@
#include "commands.h"
#include "rules.h"
#include "version.h"
+#include "HeishaModbusServer.h"
can you add the ifdef here also?
—
Reply to this email directly, view it on GitHub<#996?email_source=notifications&email_token=AAX5JPJZWZXFIXJ6ZO6PW2D5RX3A3A5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTKMZXGU3TEMRQGM4KM4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2KYZTPN52GK4S7MNWGSY3L#pullrequestreview-5375722038>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AAX5JPM3WWPSYTTVFYRCI2L5RX3A3AVCNFSNUABFKJSXA33TNF2G64TZHMYTOOBYGMZTMMRVHNEXG43VMU5TKNRTGQ3TENBVGQ32C5QC>.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS<https://github.com/notifications/mobile/ios/AAX5JPMJ5HY6YEJIEJMJFNT5RX3A3A5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTKMZXGU3TEMRQGM4KM4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2KUZTPN52GK4S7NFXXG> and Android<https://github.com/notifications/mobile/android/AAX5JPMUTKCJCGGJEGIUKIL5RX3A3A5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTKMZXGU3TEMRQGM4KM4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2K4ZTPN52GK4S7MFXGI4TPNFSA>. Download it today!
You are receiving this because you authored the thread.Message ID: ***@***.***>
|
This pull request adds Modbus TCP support to HeishaMon.
The implementation allows external systems such as PLCs, Loxone, Home Assistant or other Modbus TCP clients to read HeishaMon values directly via standard Modbus registers.
Main features
I originally developed this as a separate fork and would now like to contribute the functionality back to the main HeishaMon project.
The implementation has already been tested in real operation with a Panasonic heat pump and external Modbus TCP clients.
If a direct integration into the main project is currently not desired, I would also be very happy with a reference to the fork in the README, so users who need Modbus TCP support can find it easily.
There is no urgency from my side, so I am also happy to wait if you prefer to review or consider this at a later point.
Feedback and suggestions for changes to better integrate it into the main project are very welcome.