fix(docs): align HA entity table with name-based generation, fix domain errors#157
fix(docs): align HA entity table with name-based generation, fix domain errors#157stritti wants to merge 11 commits into
Conversation
…nvention - Fix entity table Object IDs to match name-based HA generation (mode → operation_mode, pool_max_temp → max_pool_temp, hysteresis → temperature_hysteresis, etc.) - Change binary_sensor entries back to sensor (firmware uses sensors) - Remove mqtt_status entity (not in firmware for v4.0.x) - Replace climate.pool_climate → pool_thermostat - Rewrite MAC-suffix warning: clarify entity_ids derive from the name field, not from unique_id or MAC. Remove misleading 'pool_controller_a1b2c3' example. Addresses PR #136 review comment.
✅
|
| Descriptor | Linter | Files | Fixed | Errors | Warnings | Elapsed time |
|---|---|---|---|---|---|---|
| ✅ ACTION | actionlint | 9 | 0 | 0 | 0.22s | |
| ✅ BASH | bash-exec | 1 | 0 | 0 | 0.18s | |
| ✅ BASH | shellcheck | 1 | 0 | 0 | 0.18s | |
| ✅ BASH | shfmt | 1 | 0 | 0 | 0.0s | |
| ✅ C | clang-format | 1 | 0 | 0 | 0.21s | |
| ✅ C | cppcheck | 1 | 0 | 0 | 0.01s | |
| ✅ C | cpplint | 1 | 0 | 0 | 0.47s | |
| ✅ CPP | clang-format | 64 | 0 | 0 | 0.27s | |
| ✅ CPP | cppcheck | 64 | 0 | 0 | 4.41s | |
| ✅ CPP | cpplint | 64 | 0 | 0 | 3.2s | |
| ✅ EDITORCONFIG | editorconfig-checker | 236 | 0 | 0 | 0.29s | |
| ✅ JSON | jsonlint | 6 | 0 | 0 | 0.1s | |
| ✅ JSON | v8r | 6 | 0 | 0 | 2.81s | |
| markdownlint | 96 | 1 | 0 | 2.34s | ||
| ✅ YAML | yamllint | 25 | 0 | 0 | 0.84s |
Detailed Issues
⚠️ MARKDOWN / markdownlint - 1 error
docs/contactor-guide.de.md:147:38 error MD009/no-trailing-spaces Trailing spaces [Expected: 0 or 2; Actual: 1]
See detailed reports in MegaLinter artifacts
Your project could benefit from a custom flavor, which would allow you to run only the linters you need, and thus improve runtime performances. (Skip this info by defining FLAVOR_SUGGESTIONS: false)
- Documentation: Custom Flavors
- Command:
npx mega-linter-runner@9.6.0 --custom-flavor-setup --custom-flavor-linters ACTION_ACTIONLINT,BASH_EXEC,BASH_SHELLCHECK,BASH_SHFMT,C_CPPCHECK,C_CPPLINT,C_CLANG_FORMAT,CPP_CPPCHECK,CPP_CPPLINT,CPP_CLANG_FORMAT,EDITORCONFIG_EDITORCONFIG_CHECKER,JSON_JSONLINT,JSON_V8R,MARKDOWN_MARKDOWNLINT,YAML_YAMLLINT
Show us your support by starring ⭐ the repository
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6f37217144
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The NORVI AE01-R uses active-HIGH relays: HIGH = relay energized (ON) LOW = relay de-energized (OFF) The code assumed active-LOW (standard external relay modules), causing inverted behavior: setSwitch(false) wrote HIGH to the GPIO pin, which turned the NORVI relay ON instead of OFF. Fix: - Add bool activeLow parameter to RelayModuleNode constructor (default true for backward compatibility with standard relay modules) - Use polarity-aware logic in begin() and setSwitch() digitalWrite calls - Create NORVI relay nodes with activeLow=false in PoolController.cpp
- Add relay polarity explanation to both EN and DE NORVI docs - Add activeLow parameter info to build instructions - Add relay polarity row to differences comparison table - Run clang-format on RelayModuleNode files Follow-up to 6e92aa0 (fix relay polarity).
Separate constructor overloads for (pin, measurementInterval) vs (pin, bool activeLow, measurementInterval) prevent implicit int→bool conversion when passing a numeric measurement-interval argument.
…ructors (YAGNI) No caller passes measurementInterval to the constructor — it's always set via setMeasurementInterval() afterwards. Remove the dead parameter from both constructor overloads. Also add 'Tote Funktionsparameter (YAGNI)' to the clean-code skill's K3 checklist. Signed-off-by: Stephan Strittmatter <stephan@st-strittmatter.name>
…rity Adds an isolated native test binary (test/native/relay_safety) that links the REAL RelayModuleNode, RuleAuto, RuleBoost, and RuleTimer production classes against a digitalWrite-capturing GPIO mock — verifying the actual physical pin level each mode drives, not just the logical getSwitch() state. Kept separate from the main test_runner target because that target uses simplified/offset-hacked mocks for these classes; linking both real and mocked definitions of the same class names together would be an ODR violation. Coverage: - RelayModuleNode polarity for both activeLow=false (NORVI, active-HIGH) and activeLow=true (standard external relay module, active-LOW) - RuleAuto / RuleBoost / RuleTimer: solar relay is forced OFF (and the physical pin driven to the correct de-energized level) whenever the circulation pump is off - Sanity check that the pool-pump-ON path still drives the pin HIGH Verified the new suite actually catches the regression by reintroducing the exact hardcoded active-LOW bug and confirming 10 assertions fail, then reverting. Also: - mocks/Arduino.h: add digitalWrite/digitalRead pin-state capture and String::reserve()/operator+= (needed by Utils.hpp when compiling RelayModuleNode.cpp for real) - wire the new suite into native-tests.yml CI job - .gitignore: exclude the new build/ dir
Native Test Coverage
Report from native unit tests (ASan + gcov). |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 50edae01b7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- clang-format: fix comment alignment in test_relay_safety.cpp - MD009: remove trailing space in review-codex-clean-code-iot - MD058: add blank lines before tables in review and norvi-display docs - MD034: wrap bare URL in angle brackets in norvi-display doc
Native Test Coverage
Report from native unit tests (ASan + gcov). |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e5c4216d40
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Addresses PR #157 review comments: - Correct binary_sensor mqtt_status -> mqtt_connected (name 'MQTT Connected') - Correct number Object IDs to name-derived suffixes (e.g. max_pool_temp -> maximum_pool_temperature) - Remove stray code fence + duplicate paragraph in Dependencies section
Native Test Coverage
Report from native unit tests (ASan + gcov). |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5d7bd8deb6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| | `number` | `max_pool_temp` | config | Zieltemperatur Pool max. | | ||
| | `number` | `min_solar_temp` | config | Minimale Solar-Aktivierungstemperatur | | ||
| | `number` | `temperature_hysteresis` | config | Temperaturhysterese | | ||
| | `number` | `circ_temp_threshold` | config | Schwellwert temp. Filterlaufzeit | | ||
| | `number` | `circ_temp_factor` | config | Faktor temp. Filterlaufzeit | |
There was a problem hiding this comment.
This section now tells German readers that HA entity IDs come from the MQTT discovery name field, but these number rows still list abbreviated IDs such as max_pool_temp and circ_temp_threshold. The firmware publishes names like Maximum Pool Temperature and Circulation Temperature Threshold in src/MqttPublisher.cpp, so Home Assistant will create IDs like number.<prefix>_maximum_pool_temperature and number.<prefix>_circulation_temperature_threshold; users following this table will configure non-existent entities.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
✅ Fixed in 40e9fa2: Aligned German entity table with name-derived IDs (same fixes as English)
Addresses PR #157 review comment: - Correct binary_sensor mqtt_status -> mqtt_connected - Correct number Object IDs to name-derived suffixes in German docs
|
@codex review |
Native Test Coverage
Report from native unit tests (ASan + gcov). |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 40e9fa2d92
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ${TEST_SOURCES} | ||
| ) | ||
|
|
||
| add_test(NAME relay_safety_test COMMAND test_relay_safety) |
There was a problem hiding this comment.
When this new CMake project is used through CTest or IDE test discovery rather than the workflow's direct ./build/test_relay_safety invocation, this add_test is ignored because the file never calls enable_testing() or includes CTest; CMake's own help states it "only generates tests if the enable_testing() command has been invoked," and configuring this directory then running ctest -N reports 0 tests. Add enable_testing() before registering the test so the regression suite is discoverable outside this one workflow.
Useful? React with 👍 / 👎.
1 participant
Fixes für die HA-Dokumentation
Nach PR #138 (gleicher Branch) wurde die Entity-Tabelle nie an die name-basierte Entity-ID-Generierung angepasst.
Änderungen
Entity-Tabelle aktualisiert:
mode→operation_mode(name: "Operation Mode")pool_max_temp→max_pool_temp(name: "Maximum Pool Temperature")solar_min_temp→min_solar_temphysteresis→temperature_hysteresistemp_circ_threshold→circ_temp_thresholdtemp_circ_factor→circ_temp_factortemp_circ_max_runtime→circ_max_runtimepool_climate→pool_thermostat(firmware name: "Pool Thermostat")pool_sensor_found,solar_sensor_found(firmware verwendet weiterhin binary_sensor — korrigiert in Commit 6f37217)Entity-ID-Warnung überarbeitet:
name-Feld abgeleitet werden (nicht aus unique_id oder MAC)pool_controller_a1b2c3-Beispielpool_controllerist der Geräte-PrefixTabelle kompakter (leichter lesbar):
Dashboard-YAML:
Migration aktualisiert:
Review-Hinweise
src/MqttPublisher.cpp) verifiziert