Feature/rework mai6 template #854
Conversation
WalkthroughThis pull request introduces version 2.153.2 of the Changes
Sequence DiagramsequenceDiagram
participant Config as WB-MAI6 Configuration
participant Channel as Input Channel
participant Scale as Physical Value Scaler
Config->>Channel: Define Channel Parameters
Channel->>Scale: Apply Physical Value Scale
Scale-->>Channel: Return Scaled Value
Channel-->>Config: Update Configuration
Possibly related PRs
Suggested reviewers
Poem
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: 0
🧹 Nitpick comments (2)
templates/config-wb-mai6.json.jinja (2)
275-291: Add description for physical value scale parameters.While the implementation is solid, consider adding a description field to explain:
- How the scale affects measurements
- When to use different scales
- Examples of typical use cases
{ "id": "in{{ ch_num }}_physical_value_scale", "title": "physical_value_scale_title", "group": "gg_in{{ ch_num }}_p_value", "order": 1, "readonly": true, + "description": "Defines the scale multiplier for converting raw values. Choose x1 for whole numbers, x0.1 for tenths, x0.01 for hundredths, or x0.001 for thousandths precision.", "enum": [1, 10, 100, 1000],Also applies to: 612-628
913-945: Consider optimizing channel updates.The implementation creates separate channels for each scale, which could impact performance when many channels are active. Consider:
- Using a single channel with dynamic scale updates
- Adding a note about performance implications in the template description
Also applies to: 1059-1076
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
templates/config-wb-mai6.json.jinja(28 hunks)
🔇 Additional comments (3)
templates/config-wb-mai6.json.jinja (3)
53-82: Well-structured UI group organization!The new groups for value conversion and channels are logically organized and maintain a clean UI by disabling redundant titles.
1206-1209: Comprehensive translation coverage!The new translations are well-organized and maintain consistency with the existing localization style.
Also applies to: 1331-1334
293-332: Consider improving parameter visibility logic.The current implementation creates multiple min/max parameters for different scales, which could be confusing in the UI if the conditions aren't mutually exclusive. Consider:
- Adding validation to ensure only one scale's parameters are visible at a time
- Using dynamic updates to adjust the min/max ranges when scale changes
Run this script to verify the condition logic:
Also applies to: 630-669
✅ Verification successful
Visibility logic is correctly implemented - no changes needed
The current implementation ensures mutually exclusive visibility through:
- Default case (undefined scale) showing only scale=1 parameters
- Defined scales showing only matching scale parameters
- Additional type validation ensuring parameters appear only for supported types
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for potential condition overlap in scale parameters # Look for patterns where multiple scale conditions could be true simultaneously rg -U "condition.*?physical_value_scale.*?\n.*?\{.*?\}" | grep -B1 "condition"Length of output: 78
Script:
#!/bin/bash # Find all template files and check scale-related conditions echo "=== Template files ===" fd -e jinja echo -e "\n=== Scale conditions ===" rg "physical_value_scale" -A 2 -B 2 echo -e "\n=== Scale parameters ===" rg "\"scale\":" -A 2 -B 2Length of output: 91169
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
templates/config-wb-mai6.json.jinja (3)
275-291: Good implementation of physical value scaling!The scale parameter is well-designed with appropriate ranges and readonly protection. Consider adding a description field to explain the impact of each scale on the measurement range and precision.
{ "id": "in{{ ch_num }}_physical_value_scale", "title": "physical_value_scale_title", "group": "gg_in{{ ch_num }}_p_value", "order": 1, "readonly": true, + "description": "Defines the scale and precision of measurements. Higher scales provide finer precision but smaller range.", "enum": [1, 10, 100, 1000], "enum_titles": [ "x1: [-32768; 32766]",
293-332: Robust implementation of scaled min/max parameters!The implementation handles all scale factors correctly while maintaining backward compatibility. The ranges and defaults are appropriate for each scale.
Consider extracting the common condition parts into a Jinja2 macro to reduce code duplication:
+{% macro scale_condition(ch_num, value_scale) -%} +( + {%- if value_scale == 1 -%} + isDefined(in{{ ch_num }}_physical_value_scale)==0|| + {%- endif -%} + in{{ ch_num }}_physical_value_scale=={{value_scale}})&&({{ format_list(ch_num, TYPES_WITH_MIN_MAX, "in%s_type==%s") }}) +{%- endmacro %} { "id": "in{{ ch_num }}_min", ... - "condition": "( - {%- if value_scale == 1 -%} - isDefined(in{{ ch_num }}_physical_value_scale)==0|| - {%- endif -%} - in{{ ch_num }}_physical_value_scale=={{value_scale}})&&({{ format_list(ch_num, TYPES_WITH_MIN_MAX, "in%s_type==%s") }})" + "condition": "{{ scale_condition(ch_num, value_scale) }}"
Line range hint
1-1354: Excellent implementation of physical value scaling!The changes provide a robust and backward-compatible solution for scaling physical values. The implementation is well-structured with proper organization of groups, parameters, channels, and translations.
Consider documenting the following aspects in the project's technical documentation:
- The relationship between physical value scales and measurement accuracy
- Guidelines for selecting appropriate scales based on sensor types
- Impact on data resolution and storage requirements
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
templates/config-wb-mai6.json.jinja(28 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (3)
templates/config-wb-mai6.json.jinja (3)
53-82: Well-structured group organization!The new groups for physical value conversion and channels are logically organized and follow consistent naming conventions. The UI options to disable titles help reduce visual clutter in the interface.
Line range hint
747-1077: Consistent channel configuration with scale support!The channel configurations are well-aligned with the parameter changes. The group assignments and scale-specific conditions are implemented correctly.
Line range hint
1206-1334: Complete and consistent translations!All new fields have appropriate translations in both English and Russian, maintaining consistency in terminology across the interface.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
templates/config-wb-mai6.json.jinja (3)
275-291: Consider adding parameter documentation.While the implementation is correct, adding a description field to explain the purpose and impact of the physical value scale would improve user understanding. For example:
{ "id": "in{{ ch_num }}_physical_value_scale", "title": "physical_value_scale_title", "group": "gg_in{{ ch_num }}_p_value", "order": 1, "readonly": true, + "description": "Defines the scale factor for converting raw values to physical quantities. Higher scales provide better precision for smaller values.", "enum": [1, 10, 100, 1000], "enum_titles": [ "[-32768; 32766]",Also applies to: 612-628
Line range hint
747-1077: Consider reducing channel definition duplication.The channel definitions for P and N channels contain significant duplication. Consider extracting common properties into Jinja2 macros:
{% macro create_channel(name, type, format, scale, group, condition) %} { "name": "{{ name }}", "type": "{{ type }}", "reg_type": "input", "format": "{{ format }}", "scale": {{ scale }}, "group": "{{ group }}", "condition": "{{ condition }}" } {% endmacro %}This would make the template more maintainable and reduce the chance of inconsistencies.
Line range hint
1-1354: Consider template modularization for better maintainability.The template is becoming quite large and complex. Consider splitting it into smaller, focused templates:
- Create separate templates for P and N channel configurations
- Extract common parameter definitions into reusable includes
- Move translations to separate files
This would improve maintainability and make the template easier to understand and modify.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
templates/config-wb-mai6.json.jinja(28 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (3)
templates/config-wb-mai6.json.jinja (3)
53-82: Well-structured UI group organization!The new groups for physical value conversion and channels are logically organized and follow a consistent pattern for both P and N channels.
293-332: Consider handling undefined scales more robustly.The current implementation assumes scale=1 as the fallback when
physical_value_scaleis undefined. While this works, it could be more explicit:
- Consider adding a comment explaining this default behavior
- Consider adding validation to ensure
physical_value_scalecan't be undefined for other scalesThis would make the code more maintainable and prevent potential issues if the parameter structure changes in the future.
Also applies to: 630-669
Line range hint
1206-1334: Comprehensive translation coverage!All new UI elements have proper translations in both English and Russian, maintaining consistency with the existing translation patterns.
Добавил в шаблон MAI6 возможность выбирать scale для физической величины. В зависимости от значения виртуального параметра подствляются каналы/параметры с нужным scale. Тикет: https://wirenboard.youtrack.cloud/issue/FW-6
Было:
Стало:
Summary by CodeRabbit
Release Notes for wb-mqtt-serial v2.153.2
New Features
Improvements
Documentation