Add WB-MIR v3 template, update WB-MIR v2 template #825
Conversation
debian/changelog
Outdated
| @@ -1,3 +1,10 @@ | |||
| wb-mqtt-serial (2.145.1) stable; urgency=medium | |||
There was a problem hiding this comment.
| wb-mqtt-serial (2.145.1) stable; urgency=medium | |
| wb-mqtt-serial (2.146.0) stable; urgency=medium |
|
Обсудили в чатике, решили имена топиков оставить текущими, т.к. это новая версия устройства, а не не новое устройство |
There was a problem hiding this comment.
У меня ещё есть глобальный вопрос. Тут я не увидел, чтобы было указано, что новые функции (нажатия) доступны с версии такой-то. И для v2 её надо указывать, а для v3 - нет. Может, всё-таки, для них сделать разные шаблоны? Обсуди с документацией и примите решение
…e_readings_period_s" regs back (as in m1w2)
| {% set FIRST_INPUT = 1 -%} | ||
| { | ||
| "title": "WB-MIR-v.3_template_title", | ||
| "device_type": "WB-MIR v.3", |
There was a problem hiding this comment.
wb_mir_v3?
@KraPete подскажи, есть ли критерии/пожелания, как должен выглядеть device_type?
Co-authored-by: Pavel Gasheev <[email protected]>
…"RS485 responce delay" parameter translation
WalkthroughThe pull request updates the Changes
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
test/TDeviceTemplatesTest.Validate.datis excluded by!**/*.dat
📒 Files selected for processing (4)
debian/changelog(1 hunks)templates/config-wb-mir_v2.json(1 hunks)templates/config-wb-mir_v2_buttons.json.jinja(1 hunks)templates/config-wb-mir_v3.json.jinja(1 hunks)
🔇 Additional comments (4)
templates/config-wb-mir_v2_buttons.json.jinja (2)
21-21: 🛠️ Refactor suggestion
Remove unnecessary "min_request_interval" parameter
The "min_request_interval" parameter is no longer required and should be removed to keep the template clean and up-to-date.
Apply this diff to remove the parameter:
- "min_request_interval": 100,229-252: 🛠️ Refactor suggestion
Ensure consistent naming: use "ROM" instead of "BANK"
To maintain consistency across the template and avoid confusion, consider renaming "BANK" commands to "ROM", as suggested previously.
Apply this diff to rename the commands:
- "name": "Play from BANK",
+ "name": "Play from ROM",
...
- "name": "BANK -> RAM",
+ "name": "ROM -> RAM",
...
- "name": "Learn to BANK",
+ "name": "Learn to ROM",templates/config-wb-mir_v2.json (1)
5-5: Deprecation flag added correctly
The "deprecated": true property is correctly added to indicate that the WB-MIR v2 device is deprecated.
debian/changelog (1)
1-7: Changelog updates are properly formatted
The changelog entries accurately reflect the changes made, and the formatting follows the standard conventions.
| "condition": " | ||
| {%- for in_num in range(FIRST_INPUT, INPUTS_NUMBER + 1) -%} | ||
| in{{in_num}}_mode==1 | ||
| {%- if not loop.last -%} || {%- endif -%} | ||
| {%- endfor -%} | ||
| " | ||
| }, |
There was a problem hiding this comment.
Refactor multiline "condition" field to avoid JSON syntax errors
The multiline string in the "condition" field may cause JSON syntax errors, as JSON strings cannot contain unescaped newlines. Refactor the Jinja template to generate a single-line condition.
Apply this diff to fix the issue:
- "condition": "
- {%- for in_num in range(FIRST_INPUT, INPUTS_NUMBER + 1) -%}
- in{{in_num}}_mode==1
- {%- if not loop.last -%} || {%- endif -%}
- {%- endfor -%}
- "
+ "condition": "{% for in_num in range(FIRST_INPUT, INPUTS_NUMBER + 1) -%}in{{in_num}}_mode==1{% if not loop.last %} || {% endif %}{% endfor %}"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "condition": " | |
| {%- for in_num in range(FIRST_INPUT, INPUTS_NUMBER + 1) -%} | |
| in{{in_num}}_mode==1 | |
| {%- if not loop.last -%} || {%- endif -%} | |
| {%- endfor -%} | |
| " | |
| }, | |
| "condition": "{% for in_num in range(FIRST_INPUT, INPUTS_NUMBER + 1) -%}in{{in_num}}_mode==1{% if not loop.last %} || {% endif %}{% endfor %}" | |
| }, |
There was a problem hiding this comment.
Тоже ложная тревога, нормально разваорачивается
There was a problem hiding this comment.
@ad7718, спасибо за разъяснение. Я отметил, что многострочные условия в поле "condition" Jinja-шаблона разворачиваются корректно и не вызывают ошибок JSON.
✏️ Learnings added
Learnt from: ad7718
PR: wirenboard/wb-mqtt-serial#825
File: templates/config-wb-mir_v3.json.jinja:341-347
Timestamp: 2024-12-17T16:22:40.235Z
Learning: В данном проекте многострочные условия в поле `"condition"` Jinja-шаблона разворачиваются корректно и не вызывают ошибок синтаксиса JSON.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| "reg_type": "coil", | ||
| "address": {{ 5300 + rom - 1 }}, | ||
| "type": "switch", | ||
| "group": "ir_commands", | ||
| "enabled": false | ||
| }, | ||
| { | ||
| "name": "ROM{{ rom }} size", | ||
| "reg_type": "input", | ||
| "address": {{ 5400 + rom - 1 }}, | ||
| "group": "ir_commands", | ||
| "enabled": false | ||
| }, | ||
| {% set enabled = "true" if rom < 11 else "false" -%} | ||
| { | ||
| "name": "Play from ROM{{ rom }}", | ||
| "reg_type": "coil", | ||
| "address": {{ 5100 + rom - 1 }}, | ||
| "type": "pushbutton", | ||
| "group": "ir_commands", | ||
| "enabled": {{ enabled }} | ||
| }, | ||
| { | ||
| "name": "ROM{{ rom }} -> RAM", | ||
| "reg_type": "coil", | ||
| "address": {{ 5200 + rom - 1 }}, | ||
| "type": "switch", | ||
| "group": "ir_commands", | ||
| "enabled": {{ enabled }} | ||
| }, | ||
| {% endfor -%} |
There was a problem hiding this comment.
Replace string literals with boolean values for the "enabled" field
The "enabled" field is currently set using strings "true" or "false". In JSON, boolean values should be unquoted. Modify the Jinja template to assign actual boolean values to ensure correct JSON output.
Apply this diff to fix the issue:
- {% set enabled = "true" if rom < 11 else "false" -%}
+ {% set enabled = true if rom < 11 else false -%}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "reg_type": "coil", | |
| "address": {{ 5300 + rom - 1 }}, | |
| "type": "switch", | |
| "group": "ir_commands", | |
| "enabled": false | |
| }, | |
| { | |
| "name": "ROM{{ rom }} size", | |
| "reg_type": "input", | |
| "address": {{ 5400 + rom - 1 }}, | |
| "group": "ir_commands", | |
| "enabled": false | |
| }, | |
| {% set enabled = "true" if rom < 11 else "false" -%} | |
| { | |
| "name": "Play from ROM{{ rom }}", | |
| "reg_type": "coil", | |
| "address": {{ 5100 + rom - 1 }}, | |
| "type": "pushbutton", | |
| "group": "ir_commands", | |
| "enabled": {{ enabled }} | |
| }, | |
| { | |
| "name": "ROM{{ rom }} -> RAM", | |
| "reg_type": "coil", | |
| "address": {{ 5200 + rom - 1 }}, | |
| "type": "switch", | |
| "group": "ir_commands", | |
| "enabled": {{ enabled }} | |
| }, | |
| {% endfor -%} | |
| "reg_type": "coil", | |
| "address": {{ 5300 + rom - 1 }}, | |
| "type": "switch", | |
| "group": "ir_commands", | |
| "enabled": false | |
| }, | |
| { | |
| "name": "ROM{{ rom }} size", | |
| "reg_type": "input", | |
| "address": {{ 5400 + rom - 1 }}, | |
| "group": "ir_commands", | |
| "enabled": false | |
| }, | |
| {% set enabled = true if rom < 11 else false -%} | |
| { | |
| "name": "Play from ROM{{ rom }}", | |
| "reg_type": "coil", | |
| "address": {{ 5100 + rom - 1 }}, | |
| "type": "pushbutton", | |
| "group": "ir_commands", | |
| "enabled": {{ enabled }} | |
| }, | |
| { | |
| "name": "ROM{{ rom }} -> RAM", | |
| "reg_type": "coil", | |
| "address": {{ 5200 + rom - 1 }}, | |
| "type": "switch", | |
| "group": "ir_commands", | |
| "enabled": {{ enabled }} | |
| }, | |
| {% endfor -%} |
There was a problem hiding this comment.
Тут ложная тревога, кавычек в итоговом json нет, и если их убрать, то подставляются питоновские True/False, с заглавной буквы
There was a problem hiding this comment.
@ad7718, спасибо за разъяснение. Если кавычки необходимы для правильного формирования JSON, то мой комментарий можно игнорировать. Учту это в будущих проверках.
(_/)
(•_•)
(< )
✏️ Learnings added
Learnt from: ad7718
PR: wirenboard/wb-mqtt-serial#825
File: templates/config-wb-mir_v3.json.jinja:187-217
Timestamp: 2024-12-17T16:15:42.530Z
Learning: В шаблонах Jinja при генерации JSON, использование строковых литералов "true" или "false" в кавычках позволяет избежать вставки Python True/False с заглавной буквы, что не соответствует формату JSON. Поэтому использование кавычек в таких случаях допустимо.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
|
Включил для первых 10 ROM кнопки "Learn to ROMx" для повышения удобства UI. Также теперь в шаблоне v.3 доступны все 80 ROM'ов, в v.2 оставил как было, т.к. были версии пошивок с 40 ROM'ами. |
-Старый шаблон задепрекейтил
-Добавил новый шаблон v.2 с поддержкой нажатий на jinja
-Добавил новый шаблон v.3 на jinja
-Добавил поддержку кнопок
-Убрал subdevice, 1wire/input часть сделал как в шаблоне "config-wb-m1w2-buttons.json.jinja", имена каналов 1wire/discrete input оставил такими же как в MIR v.2
Summary by CodeRabbit