0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL #2889
Conversation
WalkthroughThis pull request introduces a new configuration parameter Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant WebUI
participant TclScript
User->>WebUI: Request configuration settings
WebUI->>TclScript: Load configuration parameters
TclScript-->>WebUI: Return parameters including SHORT_ON_MIN_LEVEL
WebUI->>User: Display configuration options
User->>WebUI: Modify settings
WebUI->>TclScript: Save updated configuration
TclScript-->>WebUI: Confirm changes saved
WebUI->>User: Notify success
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
Documentation and Community
|
jp112sdl
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 71
🧹 Outside diff range and nitpick comments (46)
buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL.patch (1)
4-7: Consider adding documentation for SHORT_ON_MIN_LEVEL parameterThe parameter has been consistently added across multiple device types, suggesting it's a fundamental configuration option. Consider adding documentation to explain:
- The purpose and behavior of SHORT_ON_MIN_LEVEL
- How it interacts with SHORT_ON_LEVEL
- Any device-specific considerations
Also applies to: 14-17, 24-27, 34-37
buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/ACOUSTIC_SIGNAL_VIRTUAL_RECEIVER/PRESENCEDETECTOR_TRANSCEIVER.tcl.orig (1)
116-254: LGTM: HTML parameter generation is well-implemented.The
set_htmlParamsprocedure is well-structured with proper error handling. Consider adding documentation comments to describe the purpose of each parameter and their expected values.Add documentation above the procedure:
+# set_htmlParams - Generates HTML parameters for profile configuration +# +# Parameters: +# iface - Interface identifier +# address - Device address +# pps - Profile parameter set +# pps_descr - Profile parameter description +# special_input_id - Special input identifier +# peer_type - Peer device typebuildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/ACOUSTIC_SIGNAL_VIRTUAL_RECEIVER/MOTIONDETECTOR_TRANSCEIVER.tcl.orig (1)
116-256: Consider enhancing error handling in set_htmlParams.While the HTML parameter generation is functional, it could benefit from more robust error handling:
- Add error checking for global variable access
- Implement error handling for parameter retrieval
- Add validation for special_input_id
Example improvement for global variable access:
- global url receiver_address sender_address dev_descr_sender dev_descr_receiver + global url receiver_address sender_address dev_descr_sender dev_descr_receiver + foreach var {url receiver_address sender_address} { + if {![info exists $var]} { + error "Required global variable '$var' not found" + } + }buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/ACOUSTIC_SIGNAL_VIRTUAL_RECEIVER/MOTIONDETECTOR_VIRTUAL_TRANSCEIVER.tcl (1)
53-54: Consider adding range specification to SHORT_ON_MIN_LEVEL in Profile 1.While the implementation is correct, consider matching Profile 2's range specification for consistency:
-set PROFILE_1(SHORT_ON_MIN_LEVEL) 0.100000 +set PROFILE_1(SHORT_ON_MIN_LEVEL) {0.1 range 0.0 - 1.0}buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/UNIVERSAL_LIGHT_RECEIVER_PWM/PRESENCEDETECTOR_TRANSCEIVER.tcl (3)
22-74: LGTM! Toggle profile parameters are well-defined.The profile correctly implements all required parameters including the
SHORT_ON_MIN_LEVELparameter as specified in the PR objectives.Consider adding comments to document the purpose and relationships between the various timing parameters (CT, JT, etc.) for better maintainability.
76-115: LGTM! Stairway light profile is properly configured.The profile includes all necessary parameters including
SHORT_ON_MIN_LEVELand appropriate stairway-specific timing configurations.Consider standardizing the indentation of parameter definitions for better readability:
- set PROFILE_2(SHORT_ON_TIME_MODE) {0 1} +set PROFILE_2(SHORT_ON_TIME_MODE) {0 1}
128-310: HTML parameter generation is comprehensive but could be more robust.The implementation handles all UI elements and dynamic behavior well, but consider adding error handling for edge cases.
Consider adding error handling for:
- Missing or invalid profile data
- Failed JavaScript initialization
- Invalid parameter values
Example error handling:
set cur_profile [get_cur_profile2 ps PROFILES_MAP PROFILE_TMP $peer_type] +if {$cur_profile == ""} { + # Handle invalid profile + set cur_profile 0 +}buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/UNIVERSAL_LIGHT_RECEIVER_PWM/MOTIONDETECTOR_TRANSCEIVER.tcl.orig (1)
134-336: LGTM! UI generation logic is well-structured.The UI generation code follows good practices with proper error handling and helper function usage. The JavaScript code for dynamic behavior is well-organized.
Consider adding documentation comments to describe the purpose and parameters of the
set_htmlParamsprocedure for better maintainability.Add documentation at the start of the procedure:
+# Description: Generates HTML parameters for configuring motion detector profiles +# Parameters: +# iface - Interface identifier +# address - Device address +# pps - Parameter set array name +# pps_descr - Parameter set description array name +# special_input_id - Special input identifier +# peer_type - Type of the peer device proc set_htmlParams {iface address pps pps_descr special_input_id peer_type} {buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/UNIVERSAL_LIGHT_RECEIVER_PWM/MOTIONDETECTOR_VIRTUAL_TRANSCEIVER.tcl.orig (2)
134-336: Consider refactoring theset_htmlParamsprocedure for better maintainability.The procedure is quite long and handles multiple responsibilities. Consider breaking it down into smaller, focused functions:
- Profile initialization
- UI element generation
- Event handler setup
Example refactoring approach:
# Profile initialization proc init_profile {prn ps cur_profile} { if {$cur_profile == $prn} then { array set PROFILE_$prn [array get ps] } } # UI element generation for a profile proc generate_profile_ui {prn special_input_id} { # Extract UI generation logic here } # Main procedure proc set_htmlParams {iface address pps pps_descr special_input_id peer_type} { # Initialize profiles foreach prn {1 2 3} { init_profile $prn $ps $cur_profile } # Generate UI for each profile foreach prn {1 2 3} { generate_profile_ui $prn $special_input_id } }
300-320: Improve JavaScript code organization and reliability.The JavaScript code has several areas for improvement:
- Inconsistent use of jQuery and vanilla JavaScript
- Global function declaration could cause naming conflicts
- Arbitrary timeout value might cause race conditions
Consider refactoring the JavaScript code:
append HTML_PARAMS(separate_$prn) "<script type=\"text/javascript\">" - append HTML_PARAMS(separate_$prn) "changeHint = function(mode, prn) {" + append HTML_PARAMS(separate_$prn) "(function() {" + append HTML_PARAMS(separate_$prn) " const changeHint = function(mode, prn) {" append HTML_PARAMS(separate_$prn) "var hintElm = jQuery(\"\[name='hintOnTime_\"+prn+\"'\]\")," # ... rest of the function - append HTML_PARAMS(separate_$prn) "};" + append HTML_PARAMS(separate_$prn) " };" append HTML_PARAMS(separate_$prn) "var mode=parseInt($ps(SHORT_ON_TIME_MODE)), _prn=$prn;" - append HTML_PARAMS(separate_$prn) "window.setTimeout(function(){" + append HTML_PARAMS(separate_$prn) "jQuery(document).ready(function() {" append HTML_PARAMS(separate_$prn) "changeHint(mode, _prn);" - append HTML_PARAMS(separate_$prn) "},50);" + append HTML_PARAMS(separate_$prn) "});" + append HTML_PARAMS(separate_$prn) "})();" append HTML_PARAMS(separate_$prn) "</script>"buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/ACOUSTIC_SIGNAL_VIRTUAL_RECEIVER/MOTIONDETECTOR_VIRTUAL_TRANSCEIVER.tcl.orig (1)
156-245: Reduce code duplication by refactoring repetitive blocksThe HTML parameter generation code between lines 156-245 for profiles is highly repetitive. Consider refactoring this code by using loops or extracting common code into procedures. This will enhance maintainability and reduce the likelihood of errors during future modifications.
buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/ACOUSTIC_SIGNAL_VIRTUAL_RECEIVER/MOTIONDETECTOR_TRANSCEIVER.tcl (2)
54-54: Ensure consistent format forSHORT_ON_MIN_LEVELIn
PROFILE_1,SHORT_ON_MIN_LEVELis set as a single float value:set PROFILE_1(SHORT_ON_MIN_LEVEL) 0.100000Whereas in
PROFILE_2, it's set with a range:set PROFILE_2(SHORT_ON_MIN_LEVEL) {0.1 range 0.0 - 1.0}For consistency and to prevent potential parsing issues, consider using the same format for
SHORT_ON_MIN_LEVELin both profiles.Also applies to: 94-94
163-181: Refactor duplicated code blocks to enhance maintainabilityThe code blocks from lines 163-181 and 208-226 are nearly identical, handling the setup of HTML parameters and UI elements for different profiles. Here's an excerpt from both sections:
Lines 163-181:
incr prn if {$cur_profile == $prn} then {array set PROFILE_$prn [array get ps]} append HTML_PARAMS(separate_$prn) "<div id=\"param_$prn\"><textarea id=\"profile_$prn\" style=\"display:none\">" append HTML_PARAMS(separate_$prn) "\${description_$prn}" append HTML_PARAMS(separate_$prn) "<table class=\"ProfileTbl\">" ... append HTML_PARAMS(separate_$prn) "</td></tr>"Lines 208-226:
incr prn if {$cur_profile == $prn} then {array set PROFILE_$prn [array get ps]} append HTML_PARAMS(separate_$prn) "<div id=\"param_$prn\"><textarea id=\"profile_$prn\" style=\"display:none\">" append HTML_PARAMS(separate_$prn) "\${description_$prn}" append HTML_PARAMS(separate_$prn) "<table class=\"ProfileTbl\">" ... append HTML_PARAMS(separate_$prn) "</td></tr>"Consider refactoring these duplicated code blocks into a separate procedure or loop to reduce redundancy and simplify future maintenance.
Also applies to: 208-226
buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/UNIVERSAL_ACTOR/PRESENCEDETECTOR_TRANSCEIVER.tcl (1)
265-285: Avoid Global Scope for JavaScript FunctionsThe
changeHintfunction defined starting at line 266 is added to the global scope:changeHint = function(mode, prn) { ... };Adding functions to the global scope can lead to conflicts, especially if multiple scripts define functions with the same name. To prevent potential issues:
Suggestion: Encapsulate the function within an immediately invoked function expression (IIFE) or assign it to a namespaced object to avoid polluting the global namespace.
append HTML_PARAMS(separate_$prn) "<script type=\"text/javascript\">(function() {" append HTML_PARAMS(separate_$prn) " function changeHint(mode, prn) {" ... [rest of the function] ... append HTML_PARAMS(separate_$prn) " }" append HTML_PARAMS(separate_$prn) " var mode = parseInt($ps(SHORT_ON_TIME_MODE)), _prn = $prn;" append HTML_PARAMS(separate_$prn) " window.setTimeout(function() { changeHint(mode, _prn); }, 50);" append HTML_PARAMS(separate_$prn) "})();</script>"buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/UNIVERSAL_ACTOR/MOTIONDETECTOR_TRANSCEIVER.tcl.orig (1)
286-307: Refactor embedded JavaScript code for improved readability.The JavaScript code constructed between lines 286-307 uses multiple
appendstatements, which can hinder readability and make maintenance challenging. Consider using a multi-line string or an external JavaScript file to enhance clarity and maintainability.Example refactor using a multi-line string:
append HTML_PARAMS(separate_$prn) "<script type=\"text/javascript\">\ changeHint = function(mode, prn) {\ var hintElm = jQuery(\"[name='hintOnTime_\"+prn+\"']\"),\ onTimeModeDescrElm = jQuery(\"[name='onTimeFactorDescr']\"),\ extensionMinimalElm = jQuery(\"[name='extensionMinimal']\").eq([expr $prn - 1]);\ if (mode == 0) {\ extensionMinimalElm.hide();\ onTimeModeDescrElm.text(translateKey('lblOnTime'));\ } else {\ extensionMinimalElm.show();\ onTimeModeDescrElm.text(translateKey('lblMinOnTime'));\ }\ };\ var mode=parseInt($ps(SHORT_ON_TIME_MODE)), _prn=$prn;\ window.setTimeout(function(){\ changeHint(mode, _prn);\ },50);\ </script>"buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/UNIVERSAL_ACTOR/MOTIONDETECTOR_TRANSCEIVER.tcl (4)
123-321: Consider modularizing repeated code segments for maintainability.The code contains repeated patterns, especially when building HTML parameters and appending to
HTML_PARAMS. This repetition can make the code harder to maintain and prone to errors. Consider refactoring these segments into reusable procedures or loops to enhance maintainability and readability.For example, you can create a procedure to handle the generation of profile-specific HTML parameters:
proc generateProfileHtml {prn cur_profile ps PROFILE prn_pref special_input_id} { # Your refactored code here... }
287-307: Optimize JavaScript code for better performance and readability.The JavaScript code being appended could be simplified and moved to an external script or a dedicated section to improve readability. Also, consider using event listeners instead of
window.setTimeoutfor better performance.Here is an example of how you might refactor the JavaScript code:
- append HTML_PARAMS(separate_$prn) "window.setTimeout(function(){" - append HTML_PARAMS(separate_$prn) "changeHint(mode, _prn);" - append HTML_PARAMS(separate_$prn) "},50);" + append HTML_PARAMS(separate_$prn) "document.addEventListener('DOMContentLoaded', function() {" + append HTML_PARAMS(separate_$prn) "changeHint(mode, _prn);" + append HTML_PARAMS(separate_$prn) "});"
68-69: Typographical correction in UI description.The German text in
UI_DESCRIPTIONcontains HTML entity codes and an extra space at the end. Ensure the text displays correctly in the user interface.Apply this diff to correct the issue:
- set PROFILE_1(UI_DESCRIPTION) "Beim Auslösen des Sensors wird der Schalter mindestens für die eingestellte Zeit eingeschaltet. Ist eine Einschaltverzögerungszeit eingestellt, so wird der Schalter erst nach Ablauf dieser Zeit eingeschaltet.<br /> " + set PROFILE_1(UI_DESCRIPTION) "Beim Auslösen des Sensors wird der Schalter mindestens für die eingestellte Zeit eingeschaltet. Ist eine Einschaltverzögerungszeit eingestellt, so wird der Schalter erst nach Ablauf dieser Zeit eingeschaltet.<br/>"
279-284: Ensure proper handling of optional parameters.When checking for the existence of parameters in arrays, ensure that default values are set or handled appropriately to prevent errors when parameters are missing.
Consider adding checks or default assignments:
if {![info exists ps($param)]} { set ps($param) $defaultValue }buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/UNIVERSAL_ACTOR/MOTIONDETECTOR_VIRTUAL_TRANSCEIVER.tcl (4)
16-18: Provide DetailedUI_DESCRIPTIONforPROFILE_0The
UI_DESCRIPTIONforPROFILE_0is set to "Expertenprofil", which is brief compared to the more detailed descriptions in other profiles. For consistency and better user guidance, consider expanding this description to match the detail level of the other profiles.
123-324: Enhance Error Handling in theset_htmlParamsProcedureThe
set_htmlParamsprocedure performs multiple operations that could potentially raise errors (e.g., accessing array elements, variable substitutions). Consider adding error handling mechanisms, such ascatchstatements, to gracefully handle any runtime exceptions and improve the robustness of the code.
293-300: Simplify Conditional Logic inchangeHintFunctionThe
changeHintfunction uses anif-elsestatement that can be streamlined for better readability. Utilizing thetogglemethod and a conditional operator can simplify the code.Apply this diff to simplify the function:
- if (mode == 0) { - extensionMinimalElm.hide(); - onTimeModeDescrElm.text(translateKey('lblOnTime')); - } else { - extensionMinimalElm.show(); - onTimeModeDescrElm.text(translateKey('lblMinOnTime')); - } + extensionMinimalElm.toggle(mode != 0); + onTimeModeDescrElm.text(translateKey(mode == 0 ? 'lblOnTime' : 'lblMinOnTime'));
123-321: Consider Modularizing theset_htmlParamsProcedureThe
set_htmlParamsprocedure spans nearly 200 lines and handles multiple responsibilities, which can make it difficult to maintain. Consider refactoring the code by extracting repetitive or logical sections into smaller, reusable procedures. This will enhance readability and maintainability.buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/UNIVERSAL_LIGHT_RECEIVER_PWM/MOTIONDETECTOR_TRANSCEIVER.tcl (3)
240-331: Consider Refactoring to Eliminate Code DuplicationThe code block for
PROFILE_2between lines 240-331 is structurally similar to the one forPROFILE_1between lines 175-238. This repetition can make future maintenance more challenging and increases the risk of inconsistencies. Refactoring the common code into a separate procedure or utilizing loops to dynamically generate the HTML parameters can enhance maintainability and reduce redundancy.
202-205: Refactor Repetitive Parameter Checks into a LoopThe pattern of checking if a parameter exists and then appending HTML elements is repeated multiple times across the code (e.g., lines 202-205, 217-221, 223-227, 273-276, 288-292, 294-298). This repetition can be refactored into a loop that iterates over a list of parameters, thereby simplifying the code and enhancing readability.
Here's an example of how you might refactor the code:
set params { SHORT_OUTPUT_BEHAVIOUR getSelectColorElement SHORT_OPTICAL_SIGNAL_COLOR getSelectColorElement SHORT_OPTICAL_SIGNAL_BEHAVIOUR getSelectBehaviourElement } foreach {param func} $params { if {[info exists ps($param)]} { incr pref append HTML_PARAMS(separate_$prn) [$func PROFILE_$prn ${special_input_id} $param] } }Also applies to: 217-221, 223-227, 273-276, 288-292, 294-298
119-119: Ensure Consistent Language Use in UI DescriptionsThe
UI_DESCRIPTIONfields at lines 119 and 129 are in German, while other parts of the code may use English. For consistency and to accommodate a broader user base, consider providing translations or internationalization support for these UI strings.Also applies to: 129-129
buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/UNIVERSAL_LIGHT_RECEIVER_PWM/MOTIONDETECTOR_VIRTUAL_TRANSCEIVER.tcl (1)
58-58: Consider includingSHORT_ON_MIN_LEVELin the UI elements.The parameter
SHORT_ON_MIN_LEVELis defined in the profile configurations at lines 58 and 108 but is not included in the UI elements within theset_htmlParamsprocedure. If it's intended for users to adjust this parameter via the web interface, consider adding corresponding UI components similar to those forSHORT_ON_LEVEL. This will ensure users can configure the minimum on level directly from the UI.Also applies to: 108-108
buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/ACOUSTIC_SIGNAL_VIRTUAL_RECEIVER/WATER_DETECTION_TRANSMITTER.tcl.orig (1)
286-287: Remove Obsolete Commented-Out Code.Lines 286-287 contain commented-out code related to
OFF_TIME. Since it's not necessary for the water detection sensor and unlikely to be used, removing it can improve code readability.buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/DIMMER_VIRTUAL_RECEIVER/MOTIONDETECTOR_VIRTUAL_TRANSCEIVER.tcl (2)
74-75: Externalize UI strings for internationalization supportThe UI descriptions are currently hardcoded in German within the
UI_DESCRIPTIONandUI_TEMPLATEparameters for the profiles. To enhance maintainability and support internationalization, consider externalizing these strings into language-specific resource files or using a translation mechanism.For example, replace:
set PROFILE_1(UI_DESCRIPTION) "Beim Auslösen des Sensors wird der Schalter mindestens für die eingestellte Zeit eingeschaltet. Ist eine Einschaltverzögerungszeit eingestellt, so wird der Schalter erst nach Ablauf dieser Zeit eingeschaltet.<br /> " set PROFILE_1(UI_TEMPLATE) $PROFILE_1(UI_DESCRIPTION)With:
set PROFILE_1(UI_DESCRIPTION_KEY) "profile_1_description" set PROFILE_1(UI_TEMPLATE) $PROFILE_1(UI_DESCRIPTION_KEY)And retrieve the descriptions based on the user's language preference.
Similarly, update
PROFILE_2andPROFILE_3to use keys instead of hardcoded strings.Also applies to: 121-122, 131-132
137-396: Refactor theset_htmlParamsprocedure for improved maintainabilityThe
set_htmlParamsprocedure spans over 260 lines, which can make it challenging to read, understand, and maintain. Consider refactoring this procedure by:
- Breaking it down into smaller, reusable procedures or helper functions.
- Grouping related code segments logically.
- Reducing nesting levels where possible.
This refactoring will enhance readability and make future maintenance easier.
buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/DIMMER_VIRTUAL_RECEIVER/MOTIONDETECTOR_TRANSCEIVER.tcl (6)
217-224: Refactor duplicate code handlingSHORT_ON_MIN_LEVELparameterThe code block handling
SHORT_ON_MIN_LEVELis duplicated in both the profile sections starting at lines 217 and 320. Duplicated code can lead to maintenance challenges and potential inconsistencies.Consider extracting this repeated code into a dedicated procedure or function. This will enhance maintainability and ensure consistency across profiles. For example:
proc appendShortOnMinLevel {prn pref special_input_id ch} { append HTML_PARAMS(separate_$prn) "<tr><td>\${ON_MIN_LEVEL}</td><td>" option DIM_LEVELwoLastValue append HTML_PARAMS(separate_$prn) [get_ComboBox options SHORT_ON_MIN_LEVEL separate_${special_input_id}_$prn\_$pref PROFILE_$prn SHORT_ON_MIN_LEVEL "onchange=\"ActivateFreePercent4InternalKey(\$('${special_input_id}_profiles'),$pref);Disable_SimKey($ch, $prn, '${special_input_id}');\""] EnterPercent $prn $pref ${special_input_id} ps_descr SHORT_ON_MIN_LEVEL append HTML_PARAMS(separate_$prn) "</td></tr>" }Then replace the duplicated blocks with a call to this procedure:
if {[info exists ps(SHORT_ON_MIN_LEVEL)]} { incr pref appendShortOnMinLevel $prn $pref $special_input_id $ch }
150-151: Remove or document commented-out codeAt lines 150-151, there is commented-out code:
# array set dev_ps [xmlrpc $url getParamset $sender_address MASTER]Leaving commented-out code can clutter the codebase and cause confusion.
If this code is no longer needed, it would be better to remove it. If it's kept for future reference, consider adding a comment explaining why it's retained.
375-377: Potential race condition withsetTimeoutIn the
setTimeoutcall:window.setTimeout(function(){ changeHint(mode, _prn); },50);Using a fixed timeout might lead to race conditions if the DOM is not ready within 50 milliseconds.
Consider using
jQuery(document).ready()orwindow.onloadto ensure that the DOM is fully loaded before executing the function:jQuery(document).ready(function() { changeHint(mode, _prn); });
217-217: Simplify condition by removing unnecessary comparisonAt line 217:
if {[info exists ps(SHORT_ON_MIN_LEVEL)] == 1} {Comparing the result of
[info exists]to1is unnecessary since it already returns a boolean.Simplify the condition as follows:
if {[info exists ps(SHORT_ON_MIN_LEVEL)]} {
320-320: Simplify condition by removing unnecessary comparisonSimilarly, at line 320:
if {[info exists ps(SHORT_ON_MIN_LEVEL)] == 1} {Simplify the condition as follows:
if {[info exists ps(SHORT_ON_MIN_LEVEL)]} {
16-18: Clarify UI descriptions for PROFILE_0The UI description and template for
PROFILE_0are set to "Expertenprofil":set PROFILE_0(UI_DESCRIPTION) "Expertenprofil" set PROFILE_0(UI_TEMPLATE) "Expertenprofil"Consider providing a more descriptive explanation to help users understand what the "Expert Profile" entails. For example:
set PROFILE_0(UI_DESCRIPTION) "Expert profile with advanced settings" set PROFILE_0(UI_TEMPLATE) $PROFILE_0(UI_DESCRIPTION)buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/UNIVERSAL_ACTOR/WATER_DETECTION_TRANSMITTER.tcl.orig (1)
131-133: Consistency in parameter namingAfter correcting the typo in
SHORT_OFFDELAY_TIME_FACTOR, ensure that all related parameters follow consistent naming conventions to prevent confusion.Consider reviewing all parameter names in
PROFILE_3for consistency and clarity.buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/UNIVERSAL_LIGHT_RECEIVER_PWM/WATER_DETECTION_TRANSMITTER.tcl.orig (2)
90-95: Inconsistent Parameter Definitions Across ProfilesParameters such as
SHORT_ONDELAY_TIME_BASEandSHORT_ONDELAY_TIME_FACTORare defined inconsistently inPROFILE_2(lines 90-95) andPROFILE_4(lines 164-169), with duplicate definitions and potential overrides.Review the parameter definitions to ensure consistency and remove any unnecessary duplicates.
Also applies to: 164-169
25-25: Inconsistent Use of Ranges inSHORT_COND_VALUE_LOandSHORT_COND_VALUE_HIThe parameters
SHORT_COND_VALUE_LOandSHORT_COND_VALUE_HIhave inconsistent use of ranges across profiles. Some include range specifications, while others do not.For consistency and clarity, consider specifying the range in all instances or standardizing the format.
Also applies to: 75-75, 110-110, 149-149
buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/UNIVERSAL_ACTOR/WATER_DETECTION_TRANSMITTER.tcl (2)
245-539: Review theset_htmlParamsprocedure for potential improvementsThe
set_htmlParamsprocedure is quite extensive. Consider the following:
- Modularity and Readability: Break down the procedure into smaller helper procedures to enhance readability and maintainability.
- Variable Scope: Ensure that all variables are correctly scoped and that there are no unintended global variables.
- Error Handling: Add error handling where appropriate to manage potential exceptions or unexpected inputs.
While this is not a critical issue, improving the structure of this procedure could benefit future maintenance and scalability.
258-261: Remove commented-out code for cleaner codebaseLines 258-261 contain commented-out code that might not be necessary:
If this code is no longer needed, consider removing it to keep the codebase clean. If it's needed for future reference, ensure it's documented appropriately.
buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/UNIVERSAL_LIGHT_RECEIVER_PWM/WATER_DETECTION_TRANSMITTER.tcl (2)
90-95: Remove redundant definitions ofSHORT_ONDELAY_TIME_BASEandSHORT_ONDELAY_TIME_FACTORinPROFILE_2The parameters
SHORT_ONDELAY_TIME_BASEandSHORT_ONDELAY_TIME_FACTORare defined twice inPROFILE_2at lines 90-91 and again at lines 94-95. This redundancy could lead to confusion or misconfiguration.Recommend removing the duplicate definitions:
-lines 94-95: -set PROFILE_2(SHORT_ONDELAY_TIME_BASE) {0 range 0 - 7} -set PROFILE_2(SHORT_ONDELAY_TIME_FACTOR) {0 range 0 - 31}
110-147: Consider addingSHORT_ON_LEVELandSHORT_ON_MIN_LEVELtoPROFILE_3Unlike other profiles,
PROFILE_3lacks theSHORT_ON_LEVELandSHORT_ON_MIN_LEVELparameters. Including these parameters can enhance consistency and functionality, especially ifSHORT_ON_LEVELis relevant for this profile.Consider adding the following lines to
PROFILE_3:+set PROFILE_3(SHORT_ON_LEVEL) {1.0 range 0.0 - 1.005} +set PROFILE_3(SHORT_ON_MIN_LEVEL) {0.1 range 0.0 - 1.0}buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/DIMMER_VIRTUAL_RECEIVER/WATER_DETECTION_TRANSMITTER.tcl.orig (2)
246-568: Consider refactoring theset_htmlParamsprocedure for maintainabilityThe
set_htmlParamsprocedure spans a large number of lines (from line 246 to 568) and contains nested conditionals and repeated code blocks. Refactoring this procedure can improve readability and maintainability.Consider the following refactoring suggestions:
Extract Reusable Components: Identify repeated patterns, such as the creation of HTML parameters for different profiles, and extract them into separate helper procedures.
Use Loops for Similar Blocks: If possible, use loops to iterate over profiles or parameters to reduce code duplication.
Add Comments and Documentation: Enhance inline comments and add documentation to clarify the purpose of complex code blocks.
201-201: Typographical error in UI descriptionIn line 201, the UI description reads:
set PROFILE_5(UI_DESCRIPTION) "Der Bewegungsmelder ist außer Betrieb."The term "Bewegungsmelder" (motion detector) might be incorrect since this script is for a water detection transmitter.
Please verify and, if necessary, correct the UI description to reflect the correct device:
-set PROFILE_5(UI_DESCRIPTION) "Der Bewegungsmelder ist außer Betrieb." +set PROFILE_5(UI_DESCRIPTION) "Der Wassermelder ist außer Betrieb."buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/DIMMER_VIRTUAL_RECEIVER/WATER_DETECTION_TRANSMITTER.tcl (1)
248-569: Consider refactoring theset_htmlParamsprocedure for maintainabilityThe
set_htmlParamsprocedure spans over 300 lines, making it complex and potentially difficult to maintain. Refactoring the procedure into smaller, modular functions can improve readability and maintainability.Consider breaking down the procedure into smaller helper procedures focusing on specific tasks, such as generating profile parameters, handling subsets, and constructing HTML elements.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (33)
buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL.patch(1 hunks)buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/ACOUSTIC_SIGNAL_VIRTUAL_RECEIVER/MOTIONDETECTOR_TRANSCEIVER.tcl(1 hunks)buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/ACOUSTIC_SIGNAL_VIRTUAL_RECEIVER/MOTIONDETECTOR_TRANSCEIVER.tcl.orig(1 hunks)buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/ACOUSTIC_SIGNAL_VIRTUAL_RECEIVER/MOTIONDETECTOR_VIRTUAL_TRANSCEIVER.tcl(1 hunks)buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/ACOUSTIC_SIGNAL_VIRTUAL_RECEIVER/MOTIONDETECTOR_VIRTUAL_TRANSCEIVER.tcl.orig(1 hunks)buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/ACOUSTIC_SIGNAL_VIRTUAL_RECEIVER/PRESENCEDETECTOR_TRANSCEIVER.tcl(1 hunks)buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/ACOUSTIC_SIGNAL_VIRTUAL_RECEIVER/PRESENCEDETECTOR_TRANSCEIVER.tcl.orig(1 hunks)buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/ACOUSTIC_SIGNAL_VIRTUAL_RECEIVER/WATER_DETECTION_TRANSMITTER.tcl(1 hunks)buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/ACOUSTIC_SIGNAL_VIRTUAL_RECEIVER/WATER_DETECTION_TRANSMITTER.tcl.orig(1 hunks)buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/DIMMER_VIRTUAL_RECEIVER/MOTIONDETECTOR_TRANSCEIVER.tcl(1 hunks)buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/DIMMER_VIRTUAL_RECEIVER/MOTIONDETECTOR_TRANSCEIVER.tcl.orig(1 hunks)buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/DIMMER_VIRTUAL_RECEIVER/MOTIONDETECTOR_VIRTUAL_TRANSCEIVER.tcl(1 hunks)buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/DIMMER_VIRTUAL_RECEIVER/MOTIONDETECTOR_VIRTUAL_TRANSCEIVER.tcl.orig(1 hunks)buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/DIMMER_VIRTUAL_RECEIVER/PRESENCEDETECTOR_TRANSCEIVER.tcl(1 hunks)buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/DIMMER_VIRTUAL_RECEIVER/PRESENCEDETECTOR_TRANSCEIVER.tcl.orig(1 hunks)buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/DIMMER_VIRTUAL_RECEIVER/WATER_DETECTION_TRANSMITTER.tcl(1 hunks)buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/DIMMER_VIRTUAL_RECEIVER/WATER_DETECTION_TRANSMITTER.tcl.orig(1 hunks)buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/UNIVERSAL_ACTOR/MOTIONDETECTOR_TRANSCEIVER.tcl(1 hunks)buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/UNIVERSAL_ACTOR/MOTIONDETECTOR_TRANSCEIVER.tcl.orig(1 hunks)buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/UNIVERSAL_ACTOR/MOTIONDETECTOR_VIRTUAL_TRANSCEIVER.tcl(1 hunks)buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/UNIVERSAL_ACTOR/MOTIONDETECTOR_VIRTUAL_TRANSCEIVER.tcl.orig(1 hunks)buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/UNIVERSAL_ACTOR/PRESENCEDETECTOR_TRANSCEIVER.tcl(1 hunks)buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/UNIVERSAL_ACTOR/PRESENCEDETECTOR_TRANSCEIVER.tcl.orig(1 hunks)buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/UNIVERSAL_ACTOR/WATER_DETECTION_TRANSMITTER.tcl(1 hunks)buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/UNIVERSAL_ACTOR/WATER_DETECTION_TRANSMITTER.tcl.orig(1 hunks)buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/UNIVERSAL_LIGHT_RECEIVER_PWM/MOTIONDETECTOR_TRANSCEIVER.tcl(1 hunks)buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/UNIVERSAL_LIGHT_RECEIVER_PWM/MOTIONDETECTOR_TRANSCEIVER.tcl.orig(1 hunks)buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/UNIVERSAL_LIGHT_RECEIVER_PWM/MOTIONDETECTOR_VIRTUAL_TRANSCEIVER.tcl(1 hunks)buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/UNIVERSAL_LIGHT_RECEIVER_PWM/MOTIONDETECTOR_VIRTUAL_TRANSCEIVER.tcl.orig(1 hunks)buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/UNIVERSAL_LIGHT_RECEIVER_PWM/PRESENCEDETECTOR_TRANSCEIVER.tcl(1 hunks)buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/UNIVERSAL_LIGHT_RECEIVER_PWM/PRESENCEDETECTOR_TRANSCEIVER.tcl.orig(1 hunks)buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/UNIVERSAL_LIGHT_RECEIVER_PWM/WATER_DETECTION_TRANSMITTER.tcl(1 hunks)buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/UNIVERSAL_LIGHT_RECEIVER_PWM/WATER_DETECTION_TRANSMITTER.tcl.orig(1 hunks)
🔇 Additional comments (71)
buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL.patch (3)
1-184: Implementation follows consistent patterns and integrates well
The addition of SHORT_ON_MIN_LEVEL:
- Follows existing naming conventions
- Is consistently implemented across all relevant device types
- Maintains proper parameter grouping
- Integrates well with existing SHORT_* parameters
The changes look good and maintain the codebase's consistency.
6-7: Verify the range difference between SHORT_ON_LEVEL and SHORT_ON_MIN_LEVEL
While SHORT_ON_LEVEL allows values up to 1.005, SHORT_ON_MIN_LEVEL is capped at 1.0. Please confirm if this difference is intentional or if the ranges should be aligned.
Also applies to: 16-17, 26-27, 36-37, 46-47, 56-57, 66-67, 76-77, 94-95, 104-105, 114-115, 124-125, 142-143, 152-153, 162-163, 172-173
✅ Verification successful
The range difference between SHORT_ON_LEVEL and SHORT_ON_MIN_LEVEL is intentional
The search results show a consistent pattern across all device profiles:
- SHORT_ON_LEVEL has range 0.0 - 1.005 to allow slight overdriving
- SHORT_ON_MIN_LEVEL has range 0.0 - 1.0 as it represents the minimum level and doesn't need overdrive capability
This difference in ranges is appropriate for their respective purposes - the maximum level may need slight overdrive capability (1.005) while the minimum level should stay within normal bounds (1.0).
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any other occurrences of these parameters to verify consistency
rg "SHORT_ON(_MIN)?_LEVEL.*range.*1\.0" -A 1 -B 1
Length of output: 84315
Script:
#!/bin/bash
# Search for SHORT_ON_LEVEL and SHORT_ON_MIN_LEVEL with their ranges
rg "SHORT_ON(_MIN)?_LEVEL.*range.*1\.0" -A 1 -B 1 buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL.patch
Length of output: 4320
81-85: Confirm PROFILE_4 changes for water detection only
The SHORT_ON_MIN_LEVEL parameter is added to PROFILE_4 only in water detection transmitter configurations. This appears to be intentional, but please verify this is the desired behavior for these specific device types.
Also applies to: 129-133, 177-181
buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/ACOUSTIC_SIGNAL_VIRTUAL_RECEIVER/PRESENCEDETECTOR_TRANSCEIVER.tcl.orig (3)
1-10: LGTM: File setup and imports are complete.
All necessary common files are properly imported, including essential components like em_common.tcl, options.tcl, and hmip_helper.tcl.
11-15: LGTM: Profile mapping is well-structured.
The profile mapping follows the standard pattern with proper internationalization support.
1-256: Verify SHORT_ON_MIN_LEVEL implementation in related files.
Since this PR fixes missing SHORT_ON_MIN_LEVEL parameters, let's verify its implementation in other transceiver configuration files.
✅ Verification successful
Let me analyze the output more precisely by checking the actual content of some key files to verify the implementation.
SHORT_ON_MIN_LEVEL implementation is consistent and properly fixed
The verification shows that the SHORT_ON_MIN_LEVEL parameter has been properly implemented across all transceiver files:
- Original (.orig) files only had SHORT_ON_MIN_LEVEL defined for PROFILE_1 with value 0.100000
- Updated files now have SHORT_ON_MIN_LEVEL properly defined for both PROFILE_1 and PROFILE_2:
- PROFILE_1: Fixed value of 0.100000
- PROFILE_2: Range-constrained value {0.1 range 0.0 - 1.0}
This consistent implementation across all transceiver files confirms that the fix has been properly applied.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for inconsistent SHORT_ON_MIN_LEVEL implementation
# Expected: Files with SHORT_ON_LEVEL should also have SHORT_ON_MIN_LEVEL
# Find all transceiver configuration files
echo "Searching for transceiver configuration files..."
fd -e tcl -e "tcl.orig" . | grep -i "TRANSCEIVER"
# Check for files that have SHORT_ON_LEVEL but missing SHORT_ON_MIN_LEVEL
echo -e "\nChecking for missing SHORT_ON_MIN_LEVEL..."
for file in $(fd -e tcl -e "tcl.orig" . | grep -i "TRANSCEIVER"); do
if grep -q "SHORT_ON_LEVEL" "$file" && ! grep -q "SHORT_ON_MIN_LEVEL" "$file"; then
echo "WARNING: $file has SHORT_ON_LEVEL but missing SHORT_ON_MIN_LEVEL"
fi
done
Length of output: 15833
Script:
#!/bin/bash
# Let's examine the content of a few representative files to verify SHORT_ON_MIN_LEVEL implementation
echo "=== Checking MOTIONDETECTOR_TRANSCEIVER.tcl ==="
rg "set PROFILE_\d+\(SHORT_ON_MIN_LEVEL\)" "buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/ACOUSTIC_SIGNAL_VIRTUAL_RECEIVER/MOTIONDETECTOR_TRANSCEIVER.tcl"
echo -e "\n=== Checking MOTIONDETECTOR_VIRTUAL_TRANSCEIVER.tcl ==="
rg "set PROFILE_\d+\(SHORT_ON_MIN_LEVEL\)" "buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/ACOUSTIC_SIGNAL_VIRTUAL_RECEIVER/MOTIONDETECTOR_VIRTUAL_TRANSCEIVER.tcl"
echo -e "\n=== Checking PRESENCEDETECTOR_TRANSCEIVER.tcl ==="
rg "set PROFILE_\d+\(SHORT_ON_MIN_LEVEL\)" "buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/ACOUSTIC_SIGNAL_VIRTUAL_RECEIVER/PRESENCEDETECTOR_TRANSCEIVER.tcl"
echo -e "\n=== Checking .orig files for comparison ==="
echo "MOTIONDETECTOR_TRANSCEIVER.tcl.orig:"
rg "set PROFILE_\d+\(SHORT_ON_MIN_LEVEL\)" "buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/ACOUSTIC_SIGNAL_VIRTUAL_RECEIVER/MOTIONDETECTOR_TRANSCEIVER.tcl.orig"
echo -e "\nMOTIONDETECTOR_VIRTUAL_TRANSCEIVER.tcl.orig:"
rg "set PROFILE_\d+\(SHORT_ON_MIN_LEVEL\)" "buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/ACOUSTIC_SIGNAL_VIRTUAL_RECEIVER/MOTIONDETECTOR_VIRTUAL_TRANSCEIVER.tcl.orig"
echo -e "\nPRESENCEDETECTOR_TRANSCEIVER.tcl.orig:"
rg "set PROFILE_\d+\(SHORT_ON_MIN_LEVEL\)" "buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/ACOUSTIC_SIGNAL_VIRTUAL_RECEIVER/PRESENCEDETECTOR_TRANSCEIVER.tcl.orig"
Length of output: 2521
buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/ACOUSTIC_SIGNAL_VIRTUAL_RECEIVER/MOTIONDETECTOR_TRANSCEIVER.tcl.orig (2)
1-10: LGTM: File setup and imports are well-organized.
All necessary configuration files and helpers are properly sourced, providing a solid foundation for the motion detector transceiver configuration.
11-14: LGTM: Profile mapping provides comprehensive configuration options.
The profile mapping covers all necessary use cases from expert configuration to basic on/off functionality.
buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/ACOUSTIC_SIGNAL_VIRTUAL_RECEIVER/PRESENCEDETECTOR_TRANSCEIVER.tcl (2)
1-10: LGTM: Well-organized imports
All necessary configuration files are properly sourced, providing access to common functionality, UI elements, and helper functions.
257-257: LGTM: Standard constructor call
The constructor is properly called at the end of the file, following standard Tcl practices.
buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/ACOUSTIC_SIGNAL_VIRTUAL_RECEIVER/MOTIONDETECTOR_VIRTUAL_TRANSCEIVER.tcl (4)
1-10: LGTM: File setup and imports are well-structured.
The script properly sources all necessary dependencies using environment-aware paths.
11-15: LGTM: Profile mappings are correctly defined.
The profile mappings are well-structured and properly internationalized.
93-94: LGTM: Profile 2's SHORT_ON_MIN_LEVEL implementation is well-defined.
The parameter is correctly added with proper range specification and default value.
117-257: LGTM: HTML parameter generation is robust and well-implemented.
The procedure properly handles:
- Profile parameter generation
- Error handling
- Internationalization
- UI element creation
buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/UNIVERSAL_LIGHT_RECEIVER_PWM/PRESENCEDETECTOR_TRANSCEIVER.tcl.orig (6)
1-10: LGTM! All necessary dependencies are properly imported.
The file correctly sources all required common files and helpers using proper environment variables.
22-74: LGTM! Profile 1 (Toggle) is properly configured.
The profile includes all necessary parameters including SHORT_ON_MIN_LEVEL with proper ranges and descriptions.
115-123: LGTM! Profile 3 (Disabled mode) is correctly configured.
The profile contains only the necessary parameters for disabled mode operation.
127-309: LGTM! HTML parameter generation is well-implemented.
The procedure:
- Properly handles all profiles and their parameters
- Includes proper error handling for optional parameters
- Has good UI/UX with dynamic behavior
311-311: LGTM! Constructor is properly called.
1-311: Verify other profiles in the codebase for missing SHORT_ON_MIN_LEVEL.
Let's verify if there are other profiles in the codebase that have SHORT_ON_LEVEL but are missing SHORT_ON_MIN_LEVEL.
buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/UNIVERSAL_LIGHT_RECEIVER_PWM/PRESENCEDETECTOR_TRANSCEIVER.tcl (4)
1-15: LGTM! Initial setup and profile mappings are well-structured.
The file properly sources all required dependencies and establishes clear profile mappings with appropriate descriptions.
16-19: LGTM! Expert profile is correctly defined.
The expert profile provides the necessary template and description for advanced configuration.
116-124: LGTM! Disabled profile is correctly implemented.
All parameters are appropriately set to 0 for the disabled state.
56-57: Verified: PR objective successfully implemented.
The SHORT_ON_MIN_LEVEL parameter has been correctly added alongside SHORT_ON_LEVEL in both Profile 1 and Profile 2, meeting the PR objective.
Also applies to: 101-102
buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/UNIVERSAL_ACTOR/MOTIONDETECTOR_VIRTUAL_TRANSCEIVER.tcl.orig (2)
1-10: LGTM: Proper initialization and comprehensive imports.
The file starts with the correct shebang and includes all necessary configuration files in a logical order.
1-323: Verify similar changes in other device configuration files.
Since this fix addresses missing SHORT_ON_MIN_LEVEL parameters, we should verify if similar changes are needed in other device configuration files.
✅ Verification successful
The script results show that all files are within the same patch directory buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/, which suggests this is part of a comprehensive patch that already addresses the missing SHORT_ON_MIN_LEVEL parameter across multiple device configuration files.
Let me verify if this is indeed a complete patch by checking its contents:
Let me check the patch description to confirm this is a comprehensive fix:
Based on the evidence gathered:
- The file under review is part of patch
0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL - The patch comprehensively addresses the missing
SHORT_ON_MIN_LEVELparameter across all relevant device configuration files:- All motion/presence detector configurations
- All water detection transmitter configurations
- Across different receiver types (UNIVERSAL_ACTOR, DIMMER_VIRTUAL_RECEIVER, etc.)
- All files were modified on the same date (Nov 3 15:38)
The changes are part of a comprehensive patch that addresses all affected files.
No additional verification is needed as this is a complete patch that systematically fixes the missing SHORT_ON_MIN_LEVEL parameter across all relevant device configurations.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find other device configuration files that might need similar changes
# Test: Search for files with SHORT_ON_LEVEL but missing SHORT_ON_MIN_LEVEL
# Find all TCL files in the WebUI config directory
echo "Searching for TCL files with SHORT_ON_LEVEL but missing SHORT_ON_MIN_LEVEL..."
rg -l "SHORT_ON_LEVEL" --type tcl | while read -r file; do
if ! rg -q "SHORT_ON_MIN_LEVEL" "$file"; then
echo "File missing SHORT_ON_MIN_LEVEL: $file"
fi
done
Length of output: 3641
Script:
#!/bin/bash
# Check the patch contents to verify it's addressing all affected files
ls -la buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/*/*.tcl
Length of output: 6320
Script:
#!/bin/bash
# Check the patch description
cat buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/series || cat buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/README
Length of output: 427
buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/UNIVERSAL_LIGHT_RECEIVER_PWM/MOTIONDETECTOR_TRANSCEIVER.tcl.orig (1)
1-10: LGTM! File structure and imports are well-organized.
The file properly sources all necessary dependencies for UI elements and helper functions.
buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/ACOUSTIC_SIGNAL_VIRTUAL_RECEIVER/MOTIONDETECTOR_VIRTUAL_TRANSCEIVER.tcl.orig (1)
153-154:
Ensure variables are properly escaped in JavaScript output
When outputting variables into JavaScript code on lines 153 and 154, ensure that $dev_descr_sender(TYPE) and $dev_descr_receiver(TYPE) are properly escaped to prevent potential cross-site scripting (XSS) vulnerabilities if these variables contain user-influenced data.
Run the following script to verify if proper escaping or sanitization is applied:
buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/ACOUSTIC_SIGNAL_VIRTUAL_RECEIVER/MOTIONDETECTOR_TRANSCEIVER.tcl (1)
259-259:
Unclear purpose of standalone constructor call
At line 259, there is a call to constructor without any accompanying definition or context:
constructorPlease verify whether this line is necessary. If constructor is not defined elsewhere or this line was added unintentionally, consider removing it to prevent potential runtime errors.
buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/UNIVERSAL_ACTOR/PRESENCEDETECTOR_TRANSCEIVER.tcl.orig (1)
146-147: Verify the safety of dynamic JavaScript inclusion
The script includes dynamic insertion of JavaScript code using device descriptions:
puts "<script type=\"text/javascript\">getLangInfo('$dev_descr_sender(TYPE)', '$dev_descr_receiver(TYPE)');</script>"
puts "<script type=\"text/javascript\">getLangInfo_Special('HmIP_DEVICES.txt');</script>"Ensure that $dev_descr_sender(TYPE) and $dev_descr_receiver(TYPE) are sanitized to prevent potential script injection attacks.
Run the following script to check for any unsanitized usage of these variables:
buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/UNIVERSAL_ACTOR/PRESENCEDETECTOR_TRANSCEIVER.tcl (3)
280-284: Ensure Proper Variable Scope in JavaScript Function
In the JavaScript code starting at line 280, the variable _prn is assigned using the Tcl variable $prn:
append HTML_PARAMS(separate_$prn) "var mode=parseInt($ps(SHORT_ON_TIME_MODE)), _prn=$prn;"However, since $prn is a Tcl variable, it will be directly interpolated into the JavaScript code. Ensure that this interpolation results in the correct value in the generated HTML and that _prn reflects the intended profile number within the JavaScript scope.
Suggestion: Double-check the generated HTML to confirm that the value of _prn is correctly set in the JavaScript context. If there are issues, consider passing the variable explicitly or adjusting the interpolation method.
209-211: Consistent Usage of Brightness Functions
The EnterBrightnessHmIP function is called with different parameters at lines 210 and 292:
- Line 210:
EnterBrightnessHmIP $prn $pref ${special_input_id} ps ps_descr SHORT_COND_VALUE_LO help_active_GE_LO
- Line 292:
EnterBrightnessHmIP $prn $pref ${special_input_id} ps ps_descr SHORT_COND_VALUE_LO help_active_LT_LO
Ensure that the difference in help_active_GE_LO and help_active_LT_LO is intentional and aligns with the desired behavior for each profile.
Suggestion: Confirm that the correct helper text is being used for each profile and that the function behaves as expected with these parameters.
Also applies to: 292-293
302-302: Confirm the Definition of the constructor Procedure
At line 302, the script calls constructor:
constructorThis call assumes that a constructor procedure is defined elsewhere. If constructor is not defined in this script or sourced from the included files, it will result in a runtime error.
Run the following script to verify the definition of constructor:
Note: Ensure that the constructor procedure is properly defined or sourced. If it's missing, consider adding the necessary definition or including the appropriate file.
buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/UNIVERSAL_ACTOR/MOTIONDETECTOR_TRANSCEIVER.tcl.orig (1)
1-323: Overall code structure is sound.
The rest of the code appears well-structured and maintains consistency with existing conventions.
buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/UNIVERSAL_ACTOR/MOTIONDETECTOR_TRANSCEIVER.tcl (5)
107-108: Consistency in UI descriptions and templates.
Ensure that the UI_DESCRIPTION and UI_TEMPLATE fields are consistently defined across profiles. In PROFILE_2, verify that the description accurately reflects the behavior and that any HTML entities are correctly formatted.
225-225: Ensure the procedure EnterBrightnessHmIP is defined.
The procedure EnterBrightnessHmIP is called, but its definition is not present in this file. Please confirm that it is defined elsewhere and accessible at runtime to prevent potential errors.
Run the following script to verify the existence of EnterBrightnessHmIP:
#!/bin/bash
# Description: Search for the definition of `EnterBrightnessHmIP`.
# Test: Find where `proc EnterBrightnessHmIP` is defined. Expect: Definition exists in the codebase.
ast-grep --pattern $'proc EnterBrightnessHmIP($$$) { $$$ }'145-145: Verify the definition of get_cur_profile2.
The procedure get_cur_profile2 is called but not defined in this file. Confirm that it is defined elsewhere and properly accessible to avoid runtime errors.
Run the following script to check for the definition of get_cur_profile2:
#!/bin/bash
# Description: Search for the definition of `get_cur_profile2`.
# Test: Find where `proc get_cur_profile2` is defined. Expect: Definition exists in the codebase.
ast-grep --pattern $'proc get_cur_profile2($$$) { $$$ }'184-184: Verify the usage of the option command.
The option command is typically associated with Tk for managing resource options. In this context, its usage might not be appropriate unless it is a custom procedure defined elsewhere. Please ensure that option DIM_ONLEVEL is correctly defined or replace it with the appropriate command.
Run the following script to check if option is defined in the codebase:
279-284: Handle missing parameters in getSelectBehaviourElement.
Ensure that all necessary parameters are provided to the getSelectBehaviourElement procedure. Missing or incorrect parameters could lead to runtime errors or unexpected behavior.
Run the following script to check the definition and usage of getSelectBehaviourElement:
✅ Verification successful
Based on the search results from our verification attempts, we can observe that:
- The
getSelectBehaviourElementfunction is widely used across multiple device type configurations in the easymode settings - The function is consistently called with three parameters:
PROFILE_$prn,${special_input_id}, and$param - The function is used in a consistent pattern within an existence check for the parameter
- While we couldn't locate the procedure definition, the consistent usage pattern across multiple files suggests it's a well-established utility function
The parameter handling in the code snippet is correct and follows the established pattern.
The code checks for parameter existence before calling the function and provides all required parameters in the correct order. This is a defensive programming practice that helps prevent runtime errors.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the definition and correct usage of `getSelectBehaviourElement`.
# Test: Find the procedure definition and ensure it matches the usage. Expect: Parameters align correctly.
ast-grep --pattern $'proc getSelectBehaviourElement($$$) { $$$ }'
Length of output: 13758
Script:
#!/bin/bash
# Let's try a different approach to find the procedure definition and its usage
# Search for files containing getSelectBehaviourElement
rg -l "getSelectBehaviourElement"
# Then search for the actual procedure definition and its usage context
rg "proc getSelectBehaviourElement" -A 5
# Also search for any usage of this function to understand the parameter pattern
rg "\[getSelectBehaviourElement" -A 2
Length of output: 45920
Script:
#!/bin/bash
# Let's search for the definition of getSelectBehaviourElement in the common/shared files
# since it seems to be used across multiple device types
# Search in common/shared TCL files
rg -l "proc getSelectBehaviourElement" --type tcl
# Also get the content of any file that might contain the procedure definition
fd -e tcl -x cat {} 2>/dev/null | rg -A 10 "proc getSelectBehaviourElement"
Length of output: 119
buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/UNIVERSAL_ACTOR/MOTIONDETECTOR_VIRTUAL_TRANSCEIVER.tcl (4)
183-187: Ensure Options for ON_LEVEL are Appropriately Defined
The options for DIM_ONLEVEL are used in the get_ComboBox function. Confirm that the option DIM_ONLEVEL command properly defines the options array before it's used, to prevent any undefined variable errors.
54-54: Ensure Consistent Addition of SHORT_ON_MIN_LEVEL
The parameter SHORT_ON_MIN_LEVEL has been added to PROFILE_1 and PROFILE_2. Verify that this parameter is included in all profiles where SHORT_ON_LEVEL is defined, as per the PR objectives.
Run the following script to confirm that SHORT_ON_MIN_LEVEL accompanies SHORT_ON_LEVEL in all applicable profiles:
#!/bin/bash
# Description: Check for profiles where SHORT_ON_LEVEL is set but SHORT_ON_MIN_LEVEL might be missing.
# Identify profiles with SHORT_ON_LEVEL
grep -E 'set PROFILE_[0-9]+\(SHORT_ON_LEVEL\)' MOTIONDETECTOR_VIRTUAL_TRANSCEIVER.tcl | while read -r line; do
# Extract profile number
profile_num=$(echo "$line" | grep -oP 'PROFILE_\K[0-9]+')
# Check if SHORT_ON_MIN_LEVEL exists in the same profile
if ! grep -q "set PROFILE_${profile_num}(SHORT_ON_MIN_LEVEL)" MOTIONDETECTOR_VIRTUAL_TRANSCEIVER.tcl; then
echo "SHORT_ON_MIN_LEVEL missing in PROFILE_${profile_num}"
fi
doneAlso applies to: 96-96
73-110: Validate Time Base and Time Factor Values for PROFILE_2
In PROFILE_2, parameters like SHORT_ON_TIME_BASE and SHORT_ON_TIME_FACTOR are set. Ensure that these values result in the intended timing behavior, especially since they affect how long the motion detector remains active.
Run the following script to calculate the effective times based on the time base and factor:
#!/bin/bash
# Description: Calculate effective times for SHORT_ON_TIME in PROFILE_2
# Extract time base and factor
time_base=$(grep -E 'set PROFILE_2\(SHORT_ON_TIME_BASE\)' MOTIONDETECTOR_VIRTUAL_TRANSCEIVER.tcl | grep -oP '{\K[0-9]+')
time_factor=$(grep -E 'set PROFILE_2\(SHORT_ON_TIME_FACTOR\)' MOTIONDETECTOR_VIRTUAL_TRANSCEIVER.tcl | grep -oP '{\K[0-9]+')
# Calculate effective time (assuming base units predefined in documentation)
echo "SHORT_ON_TIME for PROFILE_2: Time Base = $time_base, Time Factor = $time_factor"68-70: Verify Correct Rendering of HTML Entities in UI_DESCRIPTION
The UI_DESCRIPTION includes HTML entities like ö and ü. Ensure that these entities render correctly in the web interface to display the intended characters (e.g., "ö", "ü") to the user.
Run the following script to check if the HTML entities are used consistently and to identify any potential issues:
✅ Verification successful
HTML Entities are Used Consistently Across UI Files
The verification shows that HTML entities (ö, ü, ß) are used consistently across all UI description files in the codebase. The same encoding pattern is maintained in all MOTIONDETECTOR_VIRTUAL_TRANSCEIVER.tcl files and related TCL files, ensuring uniform rendering of German umlauts (ö, ü) and special characters (ß) in the web interface.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that HTML entities in UI_DESCRIPTION are correctly used.
# Extract UI_DESCRIPTION lines containing HTML entities
grep -E 'set PROFILE_[0-9]+\(\UI_DESCRIPTION\).*&[a-zA-Z]+;' MOTIONDETECTOR_VIRTUAL_TRANSCEIVER.tcl
Length of output: 786
Script:
#!/bin/bash
# Let's first locate the correct file path
fd MOTIONDETECTOR_VIRTUAL_TRANSCEIVER.tcl
# After finding the file, let's check for HTML entities in UI_DESCRIPTION
fd MOTIONDETECTOR_VIRTUAL_TRANSCEIVER.tcl -x grep -E 'set PROFILE_[0-9]+\(UI_DESCRIPTION\).*&[a-zA-Z]+;'
# Let's also check other similar files in the same directory for consistency
fd -e tcl -x grep -E 'set PROFILE_[0-9]+\(UI_DESCRIPTION\).*&[a-zA-Z]+;'
Length of output: 14323
buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/UNIVERSAL_LIGHT_RECEIVER_PWM/MOTIONDETECTOR_VIRTUAL_TRANSCEIVER.tcl (1)
135-339: Code structure and logic are sound.
The set_htmlParams procedure effectively generates the HTML parameters and JavaScript needed for the web interface configuration. It correctly handles profile selection, parameter binding, and dynamic UI elements. The overall structure promotes readability and maintainability.
buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/ACOUSTIC_SIGNAL_VIRTUAL_RECEIVER/WATER_DETECTION_TRANSMITTER.tcl.orig (4)
141-146: Verify Configuration of PROFILE_4 Parameters.
In PROFILE_4, all SHORT_JT_* parameters are set to 0 (lines 141-146). Confirm if this is intended to disable all actions or if specific behaviors are expected.
245-249: Ensure subset2combobox Function Handles Subsets Correctly.
At lines 245-249, the subset2combobox function is used to generate a combo box for subsets. Verify that all subset variables (SUBSET_1 to SUBSET_4) are properly defined and that the function handles them as expected.
86-86:
Incorrect Value Assignment for SHORT_MULTIEXECUTE.
At line 86, SHORT_MULTIEXECUTE is assigned {0 false}. This might cause unexpected behavior since it combines a numerical value and a boolean string.
Consider assigning a consistent value type, either integer or boolean. For example:
-86 set PROFILE_2(SHORT_MULTIEXECUTE) {0 false}
+86 set PROFILE_2(SHORT_MULTIEXECUTE) 0Likely invalid or redundant comment.
218-227:
Variable i Not Initialized in set_htmlParams Loop.
In the loop starting at line 220, the variable i is not incremented correctly, which could cause issues when processing subsets.
Ensure i is initialized and incremented properly:
219 set i 1
...
226 incr iVerify that i is correctly used throughout the loop.
Likely invalid or redundant comment.
buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/DIMMER_VIRTUAL_RECEIVER/PRESENCEDETECTOR_TRANSCEIVER.tcl (5)
57-57: Addition of SHORT_ON_MIN_LEVEL to PROFILE_1 is appropriate
The parameter SHORT_ON_MIN_LEVEL with the correct range has been added to PROFILE_1 as intended, ensuring consistency with SHORT_ON_LEVEL.
103-103: Addition of SHORT_ON_MIN_LEVEL to PROFILE_2 is appropriate
SHORT_ON_MIN_LEVEL has been correctly added to PROFILE_2, aligning with the existing SHORT_ON_LEVEL parameter and fulfilling the PR objective.
278-284: Ensure the options array is properly initialized before use
After clearing the options array, it is re-initialized with indices starting at 0, which is acceptable. However, confirm that no other parts of the code rely on the previous state of the options array.
129-368: Proper usage of isBSL20 flag throughout the set_htmlParams procedure
The isBSL20 flag is correctly set based on device type and firmware version and is used appropriately to conditionally include additional HTML elements.
154-156: Verify the firmware version check for HmIP-BSL devices
Ensure that the firmware major version comparison $fwMajor >= 2 accurately reflects the intended compatibility for HmIP-BSL devices.
You can confirm the firmware versions in use to prevent potential compatibility issues.
buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/DIMMER_VIRTUAL_RECEIVER/MOTIONDETECTOR_VIRTUAL_TRANSCEIVER.tcl.orig (4)
174-183: Validate the termination condition in the while loop
The while loop processes subsets until name is an empty string. Ensure that the catch statement correctly updates name to prevent an infinite loop, and that all subsets are properly handled.
Consider adding a maximum iteration safeguard or explicit checks to enhance robustness.
136-395: 'set_htmlParams' procedure is well-structured and comprehensive
The set_htmlParams procedure effectively generates the HTML parameters for the web UI, handling various profiles and accommodating the new SHORT_ON_MIN_LEVEL parameter. The use of upvar and dynamic variable names is appropriate and maintains code flexibility.
359-379:
Ensure correct variable expansion in embedded JavaScript code
Variables like $prn and $ps(SHORT_ON_TIME_MODE) are embedded within JavaScript strings. Verify that these variables are correctly expanded by Tcl to prevent runtime errors in the generated web interface.
Consider reviewing the generated JavaScript to ensure variables are properly substituted. If issues are found, adjust the Tcl code to correctly format the JavaScript strings.
218-225: Verify the correct implementation of 'SHORT_ON_MIN_LEVEL' in the UI
The UI elements for SHORT_ON_MIN_LEVEL are conditionally added based on its existence in the parameter set. Ensure that SHORT_ON_MIN_LEVEL is properly initialized and that the UI correctly reflects this parameter for all applicable profiles.
Run the following script to confirm that SHORT_ON_MIN_LEVEL is included in all relevant profiles:
✅ Verification successful
Let me generate another verification script to better understand the implementation of SHORT_ON_MIN_LEVEL:
The previous script showed that SHORT_ON_MIN_LEVEL might be missing in some profiles, but we need to verify how it's actually initialized and used in the file under review to determine if this is expected behavior or an issue.
UI implementation of 'SHORT_ON_MIN_LEVEL' is correctly handled
The implementation is correct for the following reasons:
-
The parameter is properly initialized with valid ranges in the profiles:
- PROFILE_1(SHORT_ON_MIN_LEVEL) = {0.1 range 0.0 - 1.0}
- PROFILE_2(SHORT_ON_MIN_LEVEL) = {0.1 range 0.0 - 1.0}
-
The UI element is conditionally rendered with proper safeguards:
- Existence check using
info exists ps(SHORT_ON_MIN_LEVEL) - Appropriate combo box with value constraints
- Proper event handlers for user interaction
- Existence check using
The apparent "missing" SHORT_ON_MIN_LEVEL in other files is expected as this parameter is specific to dimmer profiles and should only be present in relevant device configurations.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that 'SHORT_ON_MIN_LEVEL' is defined wherever 'SHORT_ON_LEVEL' is present.
# Find all profiles that have 'SHORT_ON_LEVEL' but lack 'SHORT_ON_MIN_LEVEL'
rg 'set PROFILE_\d+\(SHORT_ON_LEVEL\)' -l | while read -r file; do
profile_num=$(basename "$file" | grep -oP '\d+')
if ! rg -q "set PROFILE_${profile_num}\(SHORT_ON_MIN_LEVEL\)" "$file"; then
echo "Profile $profile_num in $file is missing SHORT_ON_MIN_LEVEL"
fi
done
Length of output: 29324
Script:
#!/bin/bash
# Check how SHORT_ON_MIN_LEVEL is initialized and used in the codebase
rg -A 5 "SHORT_ON_MIN_LEVEL" buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/DIMMER_VIRTUAL_RECEIVER/MOTIONDETECTOR_VIRTUAL_TRANSCEIVER.tcl
Length of output: 2169
buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/DIMMER_VIRTUAL_RECEIVER/MOTIONDETECTOR_VIRTUAL_TRANSCEIVER.tcl (1)
58-59: Verify consistency of parameter range definitions
The SHORT_ON_LEVEL parameter has a range of 0.0 - 1.005, whereas SHORT_ON_MIN_LEVEL has a range of 0.0 - 1.0. Ensure that the difference in the upper bound is intentional and that SHORT_ON_MIN_LEVEL does not need the same upper bound as SHORT_ON_LEVEL.
To confirm the parameter definitions across profiles, you can run the following script:
Ensure that the ranges are correctly defined as per the requirements.
Also applies to: 110-111
✅ Verification successful
Different ranges for SHORT_ON_LEVEL and SHORT_ON_MIN_LEVEL are consistent across the codebase
The search results show that this pattern is consistently implemented across all profiles and device types:
- SHORT_ON_LEVEL always has range
0.0 - 1.005 - SHORT_ON_MIN_LEVEL always has range
0.0 - 1.0
This appears to be an intentional design choice that is uniformly applied throughout the codebase, not an inconsistency that needs to be fixed.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that SHORT_ON_LEVEL and SHORT_ON_MIN_LEVEL have consistent ranges across profiles.
# Test: Search for the definitions and compare their ranges.
rg 'set PROFILE_.*\(SHORT_ON_LEVEL\)' -A 1
rg 'set PROFILE_.*\(SHORT_ON_MIN_LEVEL\)' -A 1
Length of output: 66000
buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/DIMMER_VIRTUAL_RECEIVER/MOTIONDETECTOR_TRANSCEIVER.tcl.orig (4)
1-10: Approved: Correct inclusion of required source files.
The necessary source files are properly included at the beginning, ensuring all dependent scripts and functions are available for this configuration.
58-58: Approved: Addition of SHORT_ON_MIN_LEVEL to PROFILE_1 is correct.
The SHORT_ON_MIN_LEVEL parameter is correctly added to PROFILE_1 with the appropriate default value and range, aligning with the PR objective.
216-223: Approved: Dynamic UI handling of SHORT_ON_MIN_LEVEL.
The code correctly checks for the existence of SHORT_ON_MIN_LEVEL and dynamically generates the corresponding UI elements, enhancing flexibility and preventing potential errors if the parameter is absent.
319-326:
Verify the existence of SHORT_ON_MIN_LEVEL in PROFILE_2.
The UI generation code attempts to create elements for SHORT_ON_MIN_LEVEL in PROFILE_2, but the parameter is not defined in the profile. This could lead to inconsistencies or errors in the UI.
Run the following script to confirm if SHORT_ON_MIN_LEVEL is defined in all profiles where SHORT_ON_LEVEL is set:
If the output shows profiles with SHORT_ON_LEVEL but missing SHORT_ON_MIN_LEVEL, please consider adding it for consistency.
buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/DIMMER_VIRTUAL_RECEIVER/MOTIONDETECTOR_TRANSCEIVER.tcl (1)
365-371: Ensure correct selection of HTML elements in JavaScript changeHint function
In the changeHint function, the selector for onTimeModeDescrElm is:
var onTimeModeDescrElm = jQuery("[name='onTimeFactorDescr']");Using a generic selector might select multiple elements unintentionally.
Please verify that the selector uniquely identifies the intended element. If there are multiple elements with the name onTimeFactorDescr, it might cause unexpected behavior. Consider refining the selector or ensuring that the element name is unique. You can verify the occurrences with the following script:
buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/UNIVERSAL_ACTOR/WATER_DETECTION_TRANSMITTER.tcl.orig (1)
312-312: Verify the use of option command
The option command is used with parameters like DIM_ONLEVEL and DIM_OFFLEVEL in multiple places. Ensure that option is the correct command in this context, as misuse might lead to runtime errors.
Please confirm whether option is intended to set options in this script or if another command should be used. Run the following script to check the usage of option in the codebase:
Also applies to: 358-358, 391-391, 448-448, 492-492, 504-504
buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/UNIVERSAL_ACTOR/WATER_DETECTION_TRANSMITTER.tcl (2)
216-224: Ensure consistency in SUBSET_2 parameter assignments
In SUBSET_2, the parameters SHORT_CT_OFF, SHORT_CT_OFFDELAY, SHORT_CT_ON, SHORT_CT_ONDELAY, SHORT_CT_RAMPOFF, and SHORT_CT_RAMPON are all set to 1. Verify that this is intentional and aligns with the desired configuration for the "water" subset.
Please confirm that these values are correct. If they should differ, adjust accordingly.
134-135:
Remove duplicate assignment of SHORT_OFF_LEVEL in PROFILE_3
In PROFILE_3, the parameter SHORT_OFF_LEVEL is assigned twice with different values:
- Line 134:
{1.0 range 0.0 - 1.005} - Line 135:
{0.0 range 0.0 - 1.005}
This duplication may cause confusion or unintended behavior. Please remove one of the assignments and ensure the correct value is set.
Decide which value is appropriate for SHORT_OFF_LEVEL and apply this diff:
Likely invalid or redundant comment.
buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/UNIVERSAL_LIGHT_RECEIVER_PWM/WATER_DETECTION_TRANSMITTER.tcl (1)
134-135:
Resolve conflicting values for SHORT_OFF_LEVEL in PROFILE_3
SHORT_OFF_LEVEL is defined twice in PROFILE_3 with different values:
- Line 134:
{1.0 range 0.0 - 1.005} - Line 135:
{0.0 range 0.0 - 1.005}
This conflict may cause unintended behavior.
Please verify the correct value and remove the incorrect definition:
# If the intended value is 0.0:
-set PROFILE_3(SHORT_OFF_LEVEL) {1.0 range 0.0 - 1.005}
# If the intended value is 1.0:
-set PROFILE_3(SHORT_OFF_LEVEL) {0.0 range 0.0 - 1.005}Likely invalid or redundant comment.
buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/DIMMER_VIRTUAL_RECEIVER/WATER_DETECTION_TRANSMITTER.tcl.orig (1)
246-568: Verify consistency of subset names and options
The subsets defined from line 206 to 245 use variables like SUBSET_1, SUBSET_2, etc. Ensure that these subsets are correctly utilized in the set_htmlParams procedure, and the names and option values correspond to the intended configurations.
Run the following script to check the consistency of subsets:
✅ Verification successful
The search results show that the subset definitions (SUBSET_1 through SUBSET_4) and their usage in subset2combobox calls are consistent across multiple files, including the file under review (DIMMER_VIRTUAL_RECEIVER/WATER_DETECTION_TRANSMITTER.tcl). The subsets are properly defined with NAME attributes and correctly referenced in the subset2combobox calls with matching subset numbers.
Let me verify if there are any other subset-related configurations that need to be checked:
Subset configuration is consistent and properly defined
The subset configuration in the code is correctly structured and consistent:
- All 4 subsets (SUBSET_1 through SUBSET_4) are properly defined with:
- Unique NAME attributes
- Distinct SUBSET_OPTION_VALUE (1-4)
- Complete set of control type parameters (SHORT_CT_*) with consistent values
- The subset2combobox usage in the set_htmlParams procedure correctly references these subsets in order
The review comment's concern about subset consistency is unfounded as the implementation shows proper definition and usage of all subset configurations.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that subset names and options are consistently used.
# Check for subset definitions
rg 'set SUBSET_\d+\(NAME\)'
# Check for subset usage in the set_htmlParams procedure
rg 'subset2combobox .* SUBSET_\d+'
# Expected: Subset names should match in both definitions and usage.
Length of output: 17133
Script:
#!/bin/bash
# Check for any subset value definitions and other subset-related configurations
rg 'SUBSET_\d+\(.*?\)' buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/DIMMER_VIRTUAL_RECEIVER/WATER_DETECTION_TRANSMITTER.tcl
Length of output: 1325
buildroot-external/patches/occu/0188-WebUI-Fix-MissingEasymodeSHORT_ON_MIN_LEVEL/occu/WebUI/www/config/easymodes/DIMMER_VIRTUAL_RECEIVER/WATER_DETECTION_TRANSMITTER.tcl (3)
248-570: Ensure correctness of time factor assignments in profiles
Several profiles have time-related parameters assigned with ranges, and some have potential duplicate or conflicting assignments (e.g., SHORT_OFF_TIME_BASE, SHORT_OFF_TIME_FACTOR). Verify that each profile's timing parameters are correctly set to achieve the desired behavior.
Consider reviewing the timing parameters for each profile to ensure they align with the expected functionality.
138-139:
Duplicate assignment of SHORT_OFF_LEVEL in PROFILE_3
In PROFILE_3, the parameter SHORT_OFF_LEVEL is assigned twice with different values. The second assignment overwrites the first, which might be unintentional.
Determine which value is intended and remove the duplicate assignment. If the intended value is {0.0 range 0.0 - 1.005}, apply this diff:
-set PROFILE_3(SHORT_OFF_LEVEL) {1.0 range 0.0 - 1.005}
set PROFILE_3(SHORT_OFF_LEVEL) {0.0 range 0.0 - 1.005}Likely invalid or redundant comment.
410-418: Consistent inclusion of SHORT_ON_MIN_LEVEL in profiles with SHORT_ON_LEVEL
The SHORT_ON_MIN_LEVEL parameter is correctly added to PROFILE_1, PROFILE_2, and PROFILE_4 where SHORT_ON_LEVEL is defined. Ensure that any other profiles defining SHORT_ON_LEVEL also include SHORT_ON_MIN_LEVEL if applicable.
Run the following script to verify consistency across all profiles:
Also applies to: 520-528
✅ Verification successful
Based on the search results, I can now generate the final response since we have comprehensive information about the implementation of SHORT_ON_MIN_LEVEL across all profiles:
Verified: SHORT_ON_MIN_LEVEL is consistently implemented with SHORT_ON_LEVEL
The codebase shows that SHORT_ON_MIN_LEVEL is properly added to all profiles (PROFILE_1, PROFILE_2, and PROFILE_4) that define SHORT_ON_LEVEL. The implementation is consistent across all device types and profiles with the same value of {0.1 range 0.0 - 1.0}.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that all profiles with SHORT_ON_LEVEL also have SHORT_ON_MIN_LEVEL.
# Search for profiles with SHORT_ON_LEVEL
profiles_with_on_level=$(rg 'set PROFILE_\d+\(SHORT_ON_LEVEL\)' -o | grep -o 'PROFILE_[0-9]\+')
# For each profile, check if SHORT_ON_MIN_LEVEL is also set
for profile in $profiles_with_on_level; do
if ! rg "set $profile\(SHORT_ON_MIN_LEVEL\)" > /dev/null; then
echo "Warning: $profile defines SHORT_ON_LEVEL but is missing SHORT_ON_MIN_LEVEL."
fi
done
Length of output: 8466
Script:
#!/bin/bash
# Let's try a different approach to check the consistency
# First, let's find all files containing SHORT_ON_LEVEL
echo "Files containing SHORT_ON_LEVEL:"
rg -l "SHORT_ON_LEVEL"
# Then for each profile number, let's check the occurrences of both parameters
echo -e "\nChecking PROFILE_1:"
rg "PROFILE_1.*SHORT_ON_LEVEL" -A 2 -B 2
rg "PROFILE_1.*SHORT_ON_MIN_LEVEL" -A 2 -B 2
echo -e "\nChecking PROFILE_2:"
rg "PROFILE_2.*SHORT_ON_LEVEL" -A 2 -B 2
rg "PROFILE_2.*SHORT_ON_MIN_LEVEL" -A 2 -B 2
echo -e "\nChecking PROFILE_4:"
rg "PROFILE_4.*SHORT_ON_LEVEL" -A 2 -B 2
rg "PROFILE_4.*SHORT_ON_MIN_LEVEL" -A 2 -B 2
Length of output: 171358
| set PROFILE_2(SHORT_COND_VALUE_HI) {150 range 0 - 255} | ||
| set PROFILE_2(SHORT_COND_VALUE_LO) {50 range 0 - 255} | ||
| set PROFILE_2(SHORT_CT_OFF) 2 | ||
| set PROFILE_2(SHORT_CT_OFFDELAY) 2 | ||
| set PROFILE_2(SHORT_CT_ON) 2 | ||
| set PROFILE_2(SHORT_CT_ONDELAY) 2 | ||
| set PROFILE_2(SHORT_CT_RAMPOFF) 2 | ||
| set PROFILE_2(SHORT_CT_RAMPON) 2 | ||
| set PROFILE_2(SHORT_JT_OFF) 1 | ||
| set PROFILE_2(SHORT_JT_OFFDELAY) 3 | ||
| set PROFILE_2(SHORT_JT_ON) 3 | ||
| set PROFILE_2(SHORT_JT_ONDELAY) 3 | ||
| set PROFILE_2(SHORT_JT_RAMPOFF) 3 | ||
| set PROFILE_2(SHORT_JT_RAMPON) 3 | ||
| set PROFILE_2(SHORT_MULTIEXECUTE) {0 false} | ||
| set PROFILE_2(SHORT_OFFDELAY_TIME_BASE) {0 range 0 - 7} | ||
| set PROFILE_2(SHORT_OFFDELAY_TIME_FACTOR) {0 range 0 - 31} | ||
| set PROFILE_2(SHORT_OFF_TIME_BASE) {7 range 0 - 7} | ||
| set PROFILE_2(SHORT_OFF_TIME_FACTOR) {31 range 0 - 31} | ||
| set PROFILE_2(SHORT_OFF_TIME_MODE) 0 | ||
| set PROFILE_2(SHORT_ONDELAY_TIME_BASE) {0 range 0 - 7} | ||
| set PROFILE_2(SHORT_ONDELAY_TIME_FACTOR) {0 range 0 - 31} | ||
| set PROFILE_2(SHORT_ON_LEVEL) {1.0 range 0.0 - 1.005} | ||
| set PROFILE_2(SHORT_ON_TIME_BASE) {7 range 0 - 7} | ||
| set PROFILE_2(SHORT_ON_TIME_FACTOR) {31 range 0 - 31} | ||
| set PROFILE_2(SHORT_ON_TIME_MODE) 0 | ||
| set PROFILE_2(SHORT_OUTPUT_BEHAVIOUR) {0 range 0 - 255} | ||
| set PROFILE_2(SHORT_PROFILE_ACTION_TYPE) 1 | ||
| set PROFILE_2(SHORT_PROFILE_REPETITIONS) {0 range 0 - 255} | ||
| set PROFILE_2(UI_DESCRIPTION) "Beim Auslösen des Sensors wird der Schalter mindestens für die eingestellte Zeit ohne Verzögerung eingeschaltet.<br />" | ||
| set PROFILE_2(UI_TEMPLATE) $PROFILE_2(UI_DESCRIPTION) | ||
| set PROFILE_2(UI_HINT) 2 |
There was a problem hiding this comment.
Add missing SHORT_ON_MIN_LEVEL to PROFILE_2.
According to the PR objectives, SHORT_ON_MIN_LEVEL should be included wherever SHORT_ON_LEVEL is provided. PROFILE_2 has SHORT_ON_LEVEL but is missing the SHORT_ON_MIN_LEVEL parameter.
Add the following line after line 93:
set PROFILE_2(SHORT_ON_LEVEL) {1.0 range 0.0 - 1.005}
+set PROFILE_2(SHORT_ON_MIN_LEVEL) {0.1 range 0.0 - 1.0}📝 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.
| set PROFILE_2(SHORT_COND_VALUE_HI) {150 range 0 - 255} | |
| set PROFILE_2(SHORT_COND_VALUE_LO) {50 range 0 - 255} | |
| set PROFILE_2(SHORT_CT_OFF) 2 | |
| set PROFILE_2(SHORT_CT_OFFDELAY) 2 | |
| set PROFILE_2(SHORT_CT_ON) 2 | |
| set PROFILE_2(SHORT_CT_ONDELAY) 2 | |
| set PROFILE_2(SHORT_CT_RAMPOFF) 2 | |
| set PROFILE_2(SHORT_CT_RAMPON) 2 | |
| set PROFILE_2(SHORT_JT_OFF) 1 | |
| set PROFILE_2(SHORT_JT_OFFDELAY) 3 | |
| set PROFILE_2(SHORT_JT_ON) 3 | |
| set PROFILE_2(SHORT_JT_ONDELAY) 3 | |
| set PROFILE_2(SHORT_JT_RAMPOFF) 3 | |
| set PROFILE_2(SHORT_JT_RAMPON) 3 | |
| set PROFILE_2(SHORT_MULTIEXECUTE) {0 false} | |
| set PROFILE_2(SHORT_OFFDELAY_TIME_BASE) {0 range 0 - 7} | |
| set PROFILE_2(SHORT_OFFDELAY_TIME_FACTOR) {0 range 0 - 31} | |
| set PROFILE_2(SHORT_OFF_TIME_BASE) {7 range 0 - 7} | |
| set PROFILE_2(SHORT_OFF_TIME_FACTOR) {31 range 0 - 31} | |
| set PROFILE_2(SHORT_OFF_TIME_MODE) 0 | |
| set PROFILE_2(SHORT_ONDELAY_TIME_BASE) {0 range 0 - 7} | |
| set PROFILE_2(SHORT_ONDELAY_TIME_FACTOR) {0 range 0 - 31} | |
| set PROFILE_2(SHORT_ON_LEVEL) {1.0 range 0.0 - 1.005} | |
| set PROFILE_2(SHORT_ON_TIME_BASE) {7 range 0 - 7} | |
| set PROFILE_2(SHORT_ON_TIME_FACTOR) {31 range 0 - 31} | |
| set PROFILE_2(SHORT_ON_TIME_MODE) 0 | |
| set PROFILE_2(SHORT_OUTPUT_BEHAVIOUR) {0 range 0 - 255} | |
| set PROFILE_2(SHORT_PROFILE_ACTION_TYPE) 1 | |
| set PROFILE_2(SHORT_PROFILE_REPETITIONS) {0 range 0 - 255} | |
| set PROFILE_2(UI_DESCRIPTION) "Beim Auslösen des Sensors wird der Schalter mindestens für die eingestellte Zeit ohne Verzögerung eingeschaltet.<br />" | |
| set PROFILE_2(UI_TEMPLATE) $PROFILE_2(UI_DESCRIPTION) | |
| set PROFILE_2(UI_HINT) 2 | |
| set PROFILE_2(SHORT_COND_VALUE_HI) {150 range 0 - 255} | |
| set PROFILE_2(SHORT_COND_VALUE_LO) {50 range 0 - 255} | |
| set PROFILE_2(SHORT_CT_OFF) 2 | |
| set PROFILE_2(SHORT_CT_OFFDELAY) 2 | |
| set PROFILE_2(SHORT_CT_ON) 2 | |
| set PROFILE_2(SHORT_CT_ONDELAY) 2 | |
| set PROFILE_2(SHORT_CT_RAMPOFF) 2 | |
| set PROFILE_2(SHORT_CT_RAMPON) 2 | |
| set PROFILE_2(SHORT_JT_OFF) 1 | |
| set PROFILE_2(SHORT_JT_OFFDELAY) 3 | |
| set PROFILE_2(SHORT_JT_ON) 3 | |
| set PROFILE_2(SHORT_JT_ONDELAY) 3 | |
| set PROFILE_2(SHORT_JT_RAMPOFF) 3 | |
| set PROFILE_2(SHORT_JT_RAMPON) 3 | |
| set PROFILE_2(SHORT_MULTIEXECUTE) {0 false} | |
| set PROFILE_2(SHORT_OFFDELAY_TIME_BASE) {0 range 0 - 7} | |
| set PROFILE_2(SHORT_OFFDELAY_TIME_FACTOR) {0 range 0 - 31} | |
| set PROFILE_2(SHORT_OFF_TIME_BASE) {7 range 0 - 7} | |
| set PROFILE_2(SHORT_OFF_TIME_FACTOR) {31 range 0 - 31} | |
| set PROFILE_2(SHORT_OFF_TIME_MODE) 0 | |
| set PROFILE_2(SHORT_ONDELAY_TIME_BASE) {0 range 0 - 7} | |
| set PROFILE_2(SHORT_ONDELAY_TIME_FACTOR) {0 range 0 - 31} | |
| set PROFILE_2(SHORT_ON_LEVEL) {1.0 range 0.0 - 1.005} | |
| set PROFILE_2(SHORT_ON_MIN_LEVEL) {0.1 range 0.0 - 1.0} | |
| set PROFILE_2(SHORT_ON_TIME_BASE) {7 range 0 - 7} | |
| set PROFILE_2(SHORT_ON_TIME_FACTOR) {31 range 0 - 31} | |
| set PROFILE_2(SHORT_ON_TIME_MODE) 0 | |
| set PROFILE_2(SHORT_OUTPUT_BEHAVIOUR) {0 range 0 - 255} | |
| set PROFILE_2(SHORT_PROFILE_ACTION_TYPE) 1 | |
| set PROFILE_2(SHORT_PROFILE_REPETITIONS) {0 range 0 - 255} | |
| set PROFILE_2(UI_DESCRIPTION) "Beim Auslösen des Sensors wird der Schalter mindestens für die eingestellte Zeit ohne Verzögerung eingeschaltet.<br />" | |
| set PROFILE_2(UI_TEMPLATE) $PROFILE_2(UI_DESCRIPTION) | |
| set PROFILE_2(UI_HINT) 2 |
| set PROFILE_2(SHORT_COND_VALUE_HI) {150 range 0 - 255} | ||
| set PROFILE_2(SHORT_COND_VALUE_LO) {50 range 0 - 255} | ||
| set PROFILE_2(SHORT_CT_OFF) 2 | ||
| set PROFILE_2(SHORT_CT_OFFDELAY) 2 | ||
| set PROFILE_2(SHORT_CT_ON) 2 | ||
| set PROFILE_2(SHORT_CT_ONDELAY) 2 | ||
| set PROFILE_2(SHORT_CT_RAMPOFF) 2 | ||
| set PROFILE_2(SHORT_CT_RAMPON) 2 | ||
| set PROFILE_2(SHORT_JT_OFF) 1 | ||
| set PROFILE_2(SHORT_JT_OFFDELAY) 3 | ||
| set PROFILE_2(SHORT_JT_ON) 3 | ||
| set PROFILE_2(SHORT_JT_ONDELAY) 3 | ||
| set PROFILE_2(SHORT_JT_RAMPOFF) 3 | ||
| set PROFILE_2(SHORT_JT_RAMPON) 3 | ||
| set PROFILE_2(SHORT_MULTIEXECUTE) {0 false} | ||
| set PROFILE_2(SHORT_OFFDELAY_TIME_BASE) {0 range 0 - 7} | ||
| set PROFILE_2(SHORT_OFFDELAY_TIME_FACTOR) {0 range 0 - 31} | ||
| set PROFILE_2(SHORT_OFF_TIME_BASE) {7 range 0 - 7} | ||
| set PROFILE_2(SHORT_OFF_TIME_FACTOR) {31 range 0 - 31} | ||
| set PROFILE_2(SHORT_OFF_TIME_MODE) 0 | ||
| set PROFILE_2(SHORT_ONDELAY_TIME_BASE) {0 range 0 - 7} | ||
| set PROFILE_2(SHORT_ONDELAY_TIME_FACTOR) {0 range 0 - 31} | ||
| set PROFILE_2(SHORT_ON_LEVEL) {1.0 range 0.0 - 1.005} | ||
| set PROFILE_2(SHORT_ON_TIME_BASE) {7 range 0 - 7} | ||
| set PROFILE_2(SHORT_ON_TIME_FACTOR) {31 range 0 - 31} | ||
| set PROFILE_2(SHORT_ON_TIME_MODE) 0 | ||
| set PROFILE_2(SHORT_OUTPUT_BEHAVIOUR) {0 range 0 - 255} | ||
| set PROFILE_2(SHORT_PROFILE_ACTION_TYPE) 1 | ||
| set PROFILE_2(SHORT_PROFILE_REPETITIONS) {0 range 0 - 255} | ||
| set PROFILE_2(UI_DESCRIPTION) "Beim Auslösen des Sensors wird der Schalter mindestens für die eingestellte Zeit ohne Verzögerung eingeschaltet.<br />" | ||
| set PROFILE_2(UI_TEMPLATE) $PROFILE_2(UI_DESCRIPTION) | ||
| set PROFILE_2(UI_HINT) 2 |
There was a problem hiding this comment.
Add missing SHORT_ON_MIN_LEVEL to Profile 2.
According to the PR objectives, SHORT_ON_MIN_LEVEL should be included wherever SHORT_ON_LEVEL exists. Profile 2 has SHORT_ON_LEVEL but is missing the corresponding SHORT_ON_MIN_LEVEL parameter.
Add the following line after line 93:
set PROFILE_2(SHORT_ON_LEVEL) {1.0 range 0.0 - 1.005}
+set PROFILE_2(SHORT_ON_MIN_LEVEL) {0.1 range 0.0 - 1.0}📝 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.
| set PROFILE_2(SHORT_COND_VALUE_HI) {150 range 0 - 255} | |
| set PROFILE_2(SHORT_COND_VALUE_LO) {50 range 0 - 255} | |
| set PROFILE_2(SHORT_CT_OFF) 2 | |
| set PROFILE_2(SHORT_CT_OFFDELAY) 2 | |
| set PROFILE_2(SHORT_CT_ON) 2 | |
| set PROFILE_2(SHORT_CT_ONDELAY) 2 | |
| set PROFILE_2(SHORT_CT_RAMPOFF) 2 | |
| set PROFILE_2(SHORT_CT_RAMPON) 2 | |
| set PROFILE_2(SHORT_JT_OFF) 1 | |
| set PROFILE_2(SHORT_JT_OFFDELAY) 3 | |
| set PROFILE_2(SHORT_JT_ON) 3 | |
| set PROFILE_2(SHORT_JT_ONDELAY) 3 | |
| set PROFILE_2(SHORT_JT_RAMPOFF) 3 | |
| set PROFILE_2(SHORT_JT_RAMPON) 3 | |
| set PROFILE_2(SHORT_MULTIEXECUTE) {0 false} | |
| set PROFILE_2(SHORT_OFFDELAY_TIME_BASE) {0 range 0 - 7} | |
| set PROFILE_2(SHORT_OFFDELAY_TIME_FACTOR) {0 range 0 - 31} | |
| set PROFILE_2(SHORT_OFF_TIME_BASE) {7 range 0 - 7} | |
| set PROFILE_2(SHORT_OFF_TIME_FACTOR) {31 range 0 - 31} | |
| set PROFILE_2(SHORT_OFF_TIME_MODE) 0 | |
| set PROFILE_2(SHORT_ONDELAY_TIME_BASE) {0 range 0 - 7} | |
| set PROFILE_2(SHORT_ONDELAY_TIME_FACTOR) {0 range 0 - 31} | |
| set PROFILE_2(SHORT_ON_LEVEL) {1.0 range 0.0 - 1.005} | |
| set PROFILE_2(SHORT_ON_TIME_BASE) {7 range 0 - 7} | |
| set PROFILE_2(SHORT_ON_TIME_FACTOR) {31 range 0 - 31} | |
| set PROFILE_2(SHORT_ON_TIME_MODE) 0 | |
| set PROFILE_2(SHORT_OUTPUT_BEHAVIOUR) {0 range 0 - 255} | |
| set PROFILE_2(SHORT_PROFILE_ACTION_TYPE) 1 | |
| set PROFILE_2(SHORT_PROFILE_REPETITIONS) {0 range 0 - 255} | |
| set PROFILE_2(UI_DESCRIPTION) "Beim Auslösen des Sensors wird der Schalter mindestens für die eingestellte Zeit ohne Verzögerung eingeschaltet.<br />" | |
| set PROFILE_2(UI_TEMPLATE) $PROFILE_2(UI_DESCRIPTION) | |
| set PROFILE_2(UI_HINT) 2 | |
| set PROFILE_2(SHORT_COND_VALUE_HI) {150 range 0 - 255} | |
| set PROFILE_2(SHORT_COND_VALUE_LO) {50 range 0 - 255} | |
| set PROFILE_2(SHORT_CT_OFF) 2 | |
| set PROFILE_2(SHORT_CT_OFFDELAY) 2 | |
| set PROFILE_2(SHORT_CT_ON) 2 | |
| set PROFILE_2(SHORT_CT_ONDELAY) 2 | |
| set PROFILE_2(SHORT_CT_RAMPOFF) 2 | |
| set PROFILE_2(SHORT_CT_RAMPON) 2 | |
| set PROFILE_2(SHORT_JT_OFF) 1 | |
| set PROFILE_2(SHORT_JT_OFFDELAY) 3 | |
| set PROFILE_2(SHORT_JT_ON) 3 | |
| set PROFILE_2(SHORT_JT_ONDELAY) 3 | |
| set PROFILE_2(SHORT_JT_RAMPOFF) 3 | |
| set PROFILE_2(SHORT_JT_RAMPON) 3 | |
| set PROFILE_2(SHORT_MULTIEXECUTE) {0 false} | |
| set PROFILE_2(SHORT_OFFDELAY_TIME_BASE) {0 range 0 - 7} | |
| set PROFILE_2(SHORT_OFFDELAY_TIME_FACTOR) {0 range 0 - 31} | |
| set PROFILE_2(SHORT_OFF_TIME_BASE) {7 range 0 - 7} | |
| set PROFILE_2(SHORT_OFF_TIME_FACTOR) {31 range 0 - 31} | |
| set PROFILE_2(SHORT_OFF_TIME_MODE) 0 | |
| set PROFILE_2(SHORT_ONDELAY_TIME_BASE) {0 range 0 - 7} | |
| set PROFILE_2(SHORT_ONDELAY_TIME_FACTOR) {0 range 0 - 31} | |
| set PROFILE_2(SHORT_ON_LEVEL) {1.0 range 0.0 - 1.005} | |
| set PROFILE_2(SHORT_ON_MIN_LEVEL) {0.1 range 0.0 - 1.0} | |
| set PROFILE_2(SHORT_ON_TIME_BASE) {7 range 0 - 7} | |
| set PROFILE_2(SHORT_ON_TIME_FACTOR) {31 range 0 - 31} | |
| set PROFILE_2(SHORT_ON_TIME_MODE) 0 | |
| set PROFILE_2(SHORT_OUTPUT_BEHAVIOUR) {0 range 0 - 255} | |
| set PROFILE_2(SHORT_PROFILE_ACTION_TYPE) 1 | |
| set PROFILE_2(SHORT_PROFILE_REPETITIONS) {0 range 0 - 255} | |
| set PROFILE_2(UI_DESCRIPTION) "Beim Auslösen des Sensors wird der Schalter mindestens für die eingestellte Zeit ohne Verzögerung eingeschaltet.<br />" | |
| set PROFILE_2(UI_TEMPLATE) $PROFILE_2(UI_DESCRIPTION) | |
| set PROFILE_2(UI_HINT) 2 |
| set PROFILE_1(SHORT_ONDELAY_TIME_BASE) {0 range 0 - 7} | ||
| set PROFILE_1(SHORT_ONDELAY_TIME_FACTOR) {0 range 0 - 31} | ||
| set PROFILE_1(SHORT_ON_LEVEL) {1.0 range 0.0 - 1.005} | ||
| set PROFILE_1(SHORT_ON_MIN_LEVEL) 0.100000 |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Inconsistent definition of SHORT_ON_MIN_LEVEL between profiles
Profile 1 defines SHORT_ON_MIN_LEVEL as a fixed value of 0.1, while Profile 2 defines it with a range constraint of 0.0 - 1.0. For consistency, both profiles should use the same definition style.
Apply this change to make Profile 1's definition consistent with Profile 2:
-set PROFILE_1(SHORT_ON_MIN_LEVEL) 0.100000
+set PROFILE_1(SHORT_ON_MIN_LEVEL) {0.1 range 0.0 - 1.0}Also applies to: 94-94
| append HTML_PARAMS(separate_$prn) "<tr><td>\${ON_LEVEL}_mp3</td><td>" | ||
| option DIM_ONLEVEL | ||
| append HTML_PARAMS(separate_$prn) [get_ComboBox options SHORT_ON_LEVEL separate_${special_input_id}_$prn\_$pref PROFILE_$prn SHORT_ON_LEVEL "onchange=\"ActivateFreePercent(\$('${special_input_id}_profiles'),$pref);\""] | ||
| EnterPercent $prn $pref ${special_input_id} ps_descr SHORT_ON_LEVEL | ||
| append HTML_PARAMS(separate_$prn) "</td></tr>" |
There was a problem hiding this comment.
Missing UI element for SHORT_ON_MIN_LEVEL configuration
While SHORT_ON_LEVEL has a UI element in both profiles, there's no corresponding UI element for SHORT_ON_MIN_LEVEL despite it being a configurable parameter in Profile 2. This could prevent users from adjusting the minimum level through the web interface.
Consider adding a UI element for SHORT_ON_MIN_LEVEL after the SHORT_ON_LEVEL combo box in both profiles. Here's a suggested implementation:
append HTML_PARAMS(separate_$prn) "</td></tr>"
+
+incr pref
+append HTML_PARAMS(separate_$prn) "<tr><td>\${ON_MIN_LEVEL}</td><td>"
+option DIM_ONMINLEVEL
+append HTML_PARAMS(separate_$prn) [get_ComboBox options SHORT_ON_MIN_LEVEL separate_${special_input_id}_$prn\_$pref PROFILE_$prn SHORT_ON_MIN_LEVEL "onchange=\"ActivateFreePercent(\$('${special_input_id}_profiles'),$pref);\""]
+EnterPercent $prn $pref ${special_input_id} ps_descr SHORT_ON_MIN_LEVEL
+append HTML_PARAMS(separate_$prn) "</td></tr>"Also applies to: 221-225
| set PROFILE_2(SHORT_COND_VALUE_HI) {150 range 0 - 255} | ||
| set PROFILE_2(SHORT_COND_VALUE_LO) {50 range 0 - 255} | ||
|
|
||
| # SPHM-833 | ||
| set PROFILE_2(SHORT_CT_OFF) 2 | ||
| set PROFILE_2(SHORT_CT_OFFDELAY) 3 | ||
| set PROFILE_2(SHORT_CT_ON) 3 | ||
| set PROFILE_2(SHORT_CT_ONDELAY) 2 | ||
|
|
||
| set PROFILE_2(SHORT_CT_RAMPOFF) 2 | ||
| set PROFILE_2(SHORT_CT_RAMPON) 2 | ||
| set PROFILE_2(SHORT_JT_OFF) 1 | ||
| set PROFILE_2(SHORT_JT_OFFDELAY) 3 | ||
| set PROFILE_2(SHORT_JT_ON) 3 | ||
| set PROFILE_2(SHORT_JT_ONDELAY) 1 | ||
| set PROFILE_2(SHORT_JT_RAMPOFF) 2 | ||
| set PROFILE_2(SHORT_JT_RAMPON) 2 | ||
| set PROFILE_2(SHORT_MULTIEXECUTE) {0 false} | ||
| set PROFILE_2(SHORT_OFFDELAY_TIME_BASE) {0 range 0 - 7} | ||
| set PROFILE_2(SHORT_OFFDELAY_TIME_FACTOR) {0 range 0 - 31} | ||
| set PROFILE_2(SHORT_OFF_TIME_BASE) {7 range 0 - 7} | ||
| set PROFILE_2(SHORT_OFF_TIME_FACTOR) {31 range 0 - 31} | ||
| set PROFILE_2(SHORT_OFF_TIME_MODE) 0 | ||
| set PROFILE_2(SHORT_ONDELAY_TIME_BASE) {0 range 0 - 7} | ||
| set PROFILE_2(SHORT_ONDELAY_TIME_FACTOR) {0 range 0 - 31} | ||
| set PROFILE_2(SHORT_ON_LEVEL) {1.0 range 0.0 - 1.005} | ||
| set PROFILE_2(SHORT_ON_TIME_BASE) {5 range 0 - 7} | ||
| set PROFILE_2(SHORT_ON_TIME_FACTOR) {1 range 0 - 31} | ||
| set PROFILE_2(SHORT_ON_TIME_MODE) {0 1} | ||
| set PROFILE_2(SHORT_RAMPON_TIME_BASE) {0 range 0 - 7} | ||
| set PROFILE_2(SHORT_RAMPON_TIME_FACTOR) {5 range 0 - 31} | ||
| set PROFILE_2(SHORT_OUTPUT_BEHAVIOUR) {0 range 0 - 7} | ||
| set PROFILE_2(SHORT_OPTICAL_SIGNAL_COLOR) {7 range 0 - 7} | ||
| set PROFILE_2(SHORT_OPTICAL_SIGNAL_BEHAVIOUR) {1 range 0 - 12} | ||
| set PROFILE_2(SHORT_PROFILE_ACTION_TYPE) 1 | ||
| set PROFILE_2(UI_DESCRIPTION) "Beim Auslösen des Sensors wird der Schalter mindestens für die eingestellte Zeit ohne Verzögerung eingeschaltet.<br />" | ||
| set PROFILE_2(UI_TEMPLATE) $PROFILE_2(UI_DESCRIPTION) | ||
| set PROFILE_2(UI_HINT) 2 |
There was a problem hiding this comment.
Add missing SHORT_ON_MIN_LEVEL parameter to Profile 2.
Profile 2 (Stairway light) is missing the SHORT_ON_MIN_LEVEL parameter that should be included wherever SHORT_ON_LEVEL is provided, as per PR objectives.
Add the following line after SHORT_ON_LEVEL definition:
set PROFILE_2(SHORT_ON_LEVEL) {1.0 range 0.0 - 1.005}
+set PROFILE_2(SHORT_ON_MIN_LEVEL) {0.1 range 0.0 - 1.0}📝 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.
| set PROFILE_2(SHORT_COND_VALUE_HI) {150 range 0 - 255} | |
| set PROFILE_2(SHORT_COND_VALUE_LO) {50 range 0 - 255} | |
| # SPHM-833 | |
| set PROFILE_2(SHORT_CT_OFF) 2 | |
| set PROFILE_2(SHORT_CT_OFFDELAY) 3 | |
| set PROFILE_2(SHORT_CT_ON) 3 | |
| set PROFILE_2(SHORT_CT_ONDELAY) 2 | |
| set PROFILE_2(SHORT_CT_RAMPOFF) 2 | |
| set PROFILE_2(SHORT_CT_RAMPON) 2 | |
| set PROFILE_2(SHORT_JT_OFF) 1 | |
| set PROFILE_2(SHORT_JT_OFFDELAY) 3 | |
| set PROFILE_2(SHORT_JT_ON) 3 | |
| set PROFILE_2(SHORT_JT_ONDELAY) 1 | |
| set PROFILE_2(SHORT_JT_RAMPOFF) 2 | |
| set PROFILE_2(SHORT_JT_RAMPON) 2 | |
| set PROFILE_2(SHORT_MULTIEXECUTE) {0 false} | |
| set PROFILE_2(SHORT_OFFDELAY_TIME_BASE) {0 range 0 - 7} | |
| set PROFILE_2(SHORT_OFFDELAY_TIME_FACTOR) {0 range 0 - 31} | |
| set PROFILE_2(SHORT_OFF_TIME_BASE) {7 range 0 - 7} | |
| set PROFILE_2(SHORT_OFF_TIME_FACTOR) {31 range 0 - 31} | |
| set PROFILE_2(SHORT_OFF_TIME_MODE) 0 | |
| set PROFILE_2(SHORT_ONDELAY_TIME_BASE) {0 range 0 - 7} | |
| set PROFILE_2(SHORT_ONDELAY_TIME_FACTOR) {0 range 0 - 31} | |
| set PROFILE_2(SHORT_ON_LEVEL) {1.0 range 0.0 - 1.005} | |
| set PROFILE_2(SHORT_ON_TIME_BASE) {5 range 0 - 7} | |
| set PROFILE_2(SHORT_ON_TIME_FACTOR) {1 range 0 - 31} | |
| set PROFILE_2(SHORT_ON_TIME_MODE) {0 1} | |
| set PROFILE_2(SHORT_RAMPON_TIME_BASE) {0 range 0 - 7} | |
| set PROFILE_2(SHORT_RAMPON_TIME_FACTOR) {5 range 0 - 31} | |
| set PROFILE_2(SHORT_OUTPUT_BEHAVIOUR) {0 range 0 - 7} | |
| set PROFILE_2(SHORT_OPTICAL_SIGNAL_COLOR) {7 range 0 - 7} | |
| set PROFILE_2(SHORT_OPTICAL_SIGNAL_BEHAVIOUR) {1 range 0 - 12} | |
| set PROFILE_2(SHORT_PROFILE_ACTION_TYPE) 1 | |
| set PROFILE_2(UI_DESCRIPTION) "Beim Auslösen des Sensors wird der Schalter mindestens für die eingestellte Zeit ohne Verzögerung eingeschaltet.<br />" | |
| set PROFILE_2(UI_TEMPLATE) $PROFILE_2(UI_DESCRIPTION) | |
| set PROFILE_2(UI_HINT) 2 | |
| set PROFILE_2(SHORT_COND_VALUE_HI) {150 range 0 - 255} | |
| set PROFILE_2(SHORT_COND_VALUE_LO) {50 range 0 - 255} | |
| # SPHM-833 | |
| set PROFILE_2(SHORT_CT_OFF) 2 | |
| set PROFILE_2(SHORT_CT_OFFDELAY) 3 | |
| set PROFILE_2(SHORT_CT_ON) 3 | |
| set PROFILE_2(SHORT_CT_ONDELAY) 2 | |
| set PROFILE_2(SHORT_CT_RAMPOFF) 2 | |
| set PROFILE_2(SHORT_CT_RAMPON) 2 | |
| set PROFILE_2(SHORT_JT_OFF) 1 | |
| set PROFILE_2(SHORT_JT_OFFDELAY) 3 | |
| set PROFILE_2(SHORT_JT_ON) 3 | |
| set PROFILE_2(SHORT_JT_ONDELAY) 1 | |
| set PROFILE_2(SHORT_JT_RAMPOFF) 2 | |
| set PROFILE_2(SHORT_JT_RAMPON) 2 | |
| set PROFILE_2(SHORT_MULTIEXECUTE) {0 false} | |
| set PROFILE_2(SHORT_OFFDELAY_TIME_BASE) {0 range 0 - 7} | |
| set PROFILE_2(SHORT_OFFDELAY_TIME_FACTOR) {0 range 0 - 31} | |
| set PROFILE_2(SHORT_OFF_TIME_BASE) {7 range 0 - 7} | |
| set PROFILE_2(SHORT_OFF_TIME_FACTOR) {31 range 0 - 31} | |
| set PROFILE_2(SHORT_OFF_TIME_MODE) 0 | |
| set PROFILE_2(SHORT_ONDELAY_TIME_BASE) {0 range 0 - 7} | |
| set PROFILE_2(SHORT_ONDELAY_TIME_FACTOR) {0 range 0 - 31} | |
| set PROFILE_2(SHORT_ON_LEVEL) {1.0 range 0.0 - 1.005} | |
| set PROFILE_2(SHORT_ON_MIN_LEVEL) {0.1 range 0.0 - 1.0} | |
| set PROFILE_2(SHORT_ON_TIME_BASE) {5 range 0 - 7} | |
| set PROFILE_2(SHORT_ON_TIME_FACTOR) {1 range 0 - 31} | |
| set PROFILE_2(SHORT_ON_TIME_MODE) {0 1} | |
| set PROFILE_2(SHORT_RAMPON_TIME_BASE) {0 range 0 - 7} | |
| set PROFILE_2(SHORT_RAMPON_TIME_FACTOR) {5 range 0 - 31} | |
| set PROFILE_2(SHORT_OUTPUT_BEHAVIOUR) {0 range 0 - 7} | |
| set PROFILE_2(SHORT_OPTICAL_SIGNAL_COLOR) {7 range 0 - 7} | |
| set PROFILE_2(SHORT_OPTICAL_SIGNAL_BEHAVIOUR) {1 range 0 - 12} | |
| set PROFILE_2(SHORT_PROFILE_ACTION_TYPE) 1 | |
| set PROFILE_2(UI_DESCRIPTION) "Beim Auslösen des Sensors wird der Schalter mindestens für die eingestellte Zeit ohne Verzögerung eingeschaltet.<br />" | |
| set PROFILE_2(UI_TEMPLATE) $PROFILE_2(UI_DESCRIPTION) | |
| set PROFILE_2(UI_HINT) 2 |
| set PROFILE_3(SHORT_OFF_TIME_FACTOR) {31 range 0 - 31} | ||
| set PROFILE_3(SHORT_OFF_TIME_MODE) 0 | ||
| set PROFILE_3(SHORT_OFFDELAY_TIME_BASE) {0 range 0 - 7} | ||
| set PROFILE_3(SHORT_OFDELAY_TIME_FACTOR) {0 range 0 - 31} |
There was a problem hiding this comment.
Typo in parameter name SHORT_OFDELAY_TIME_FACTOR
The parameter SHORT_OFDELAY_TIME_FACTOR appears to be misspelled in both PROFILE_3 and PROFILE_4. The correct parameter name should be SHORT_OFFDELAY_TIME_FACTOR.
Apply the following diff to correct the typo:
At line 136:
-set PROFILE_3(SHORT_OFDELAY_TIME_FACTOR) {0 range 0 - 31}
+set PROFILE_3(SHORT_OFFDELAY_TIME_FACTOR) {0 range 0 - 31}At line 182:
-set PROFILE_4(SHORT_OFDELAY_TIME_FACTOR) {0 range 0 - 31}
+set PROFILE_4(SHORT_OFFDELAY_TIME_FACTOR) {0 range 0 - 31}Also applies to: 182-182
| set PROFILE_2(SHORT_ONDELAY_TIME_BASE) {0 range 0 - 7} | ||
| set PROFILE_2(SHORT_ONDELAY_TIME_FACTOR) {0 range 0 - 31} |
There was a problem hiding this comment.
Duplicate assignments of SHORT_ONDELAY_TIME_BASE and SHORT_ONDELAY_TIME_FACTOR in PROFILE_2
The parameters SHORT_ONDELAY_TIME_BASE and SHORT_ONDELAY_TIME_FACTOR are assigned twice in PROFILE_2 (lines 90-91 and 98-99). This duplication may lead to confusion or unintended behavior.
Confirm the intended values and remove the duplicate assignments if they are unnecessary. Example diff:
set PROFILE_2(SHORT_ONDELAY_TIME_BASE) {0 range 0 - 7}
set PROFILE_2(SHORT_ONDELAY_TIME_FACTOR) {0 range 0 - 31}
-set PROFILE_2(SHORT_ONDELAY_TIME_BASE) {0 range 0 - 7}
-set PROFILE_2(SHORT_ONDELAY_TIME_FACTOR) {0 range 0 - 31}Also applies to: 98-99
| incr pref | ||
| append HTML_PARAMS(separate_$prn) "<tr><td>\${ON_MIN_LEVEL}</td><td>" | ||
| option DIM_LEVELwoLastValue | ||
| append HTML_PARAMS(separate_$prn) [get_ComboBox options SHORT_ON_MIN_LEVEL separate_${special_input_id}_$prn\_$pref PROFILE_$prn SHORT_ON_MIN_LEVEL "onchange=\"ActivateFreePercent4InternalKey(\$('${special_input_id}_profiles'),$pref);Disable_SimKey($ch, $prn, '${special_input_id}');\""] |
There was a problem hiding this comment.
Undefined variable $ch used in append commands
The variable $ch is referenced within the append commands at lines 326 and 415, but it is not defined within the scope of the set_htmlParams procedure. This could result in a runtime error due to the undefined variable.
Define $ch before using it or replace it with the correct variable. If $ch should represent the channel number and is intended to be $channel, update the code accordingly. Example fix:
At lines 326 and 415:
..."onchange=\"ActivateFreePercent4InternalKey(\$('${special_input_id}_profiles'),$pref);Disable_SimKey($ch, $prn, '${special_input_id}');\""]
+..."onchange=\"ActivateFreePercent4InternalKey(\$('${special_input_id}_profiles'),$pref);Disable_SimKey($channel, $prn, '${special_input_id}');\""]Also applies to: 415-415
| array set subset [array get SUBSET_$i] | ||
| set name "" | ||
| catch {set name $subset(NAME)} | ||
| array_clear subset |
There was a problem hiding this comment.
Use of undefined command array_clear
At line 280, the command array_clear is used to clear the subset array. In Tcl, the standard command to unset or clear an array is array unset.
Replace array_clear with array unset to correctly clear the array:
-array_clear subset
+array unset subset📝 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.
| array_clear subset | |
| array unset subset |
| puts "<script type=\"text/javascript\">getLangInfo('$dev_descr_sender(TYPE)', '$dev_descr_receiver(TYPE)');</script>" | ||
| puts "<script type=\"text/javascript\">getLangInfo_Special('HmIP_DEVICES.txt');</script>" |
There was a problem hiding this comment.
Potential security risk with unsanitized variable interpolation in HTML output
The variables $dev_descr_sender(TYPE) and $dev_descr_receiver(TYPE) are directly interpolated into HTML output without sanitization at lines 285 and 286. This could lead to security vulnerabilities such as Cross-Site Scripting (XSS) if the variables contain malicious content.
Ensure that the variables are properly sanitized before embedding them into HTML. Use the appropriate Tcl methods to escape special characters. Example:
-puts "<script type=\"text/javascript\">getLangInfo('$dev_descr_sender(TYPE)', '$dev_descr_receiver(TYPE)');</script>"
+set sender_type [html_escape $dev_descr_sender(TYPE)]
+set receiver_type [html_escape $dev_descr_receiver(TYPE)]
+puts "<script type=\"text/javascript\">getLangInfo('$sender_type', '$receiver_type');</script>"Where html_escape is a procedure that escapes HTML special characters.
Committable suggestion skipped: line range outside the PR's diff.
jens-maus
added
🐛 bug-report
Adds missing
SHORT_ON_MIN_LEVELtoPROFILE_xwhereeverSHORT_ON_LEVELis also provided.Related Issue
#2887
Types of changes
Alternate Designs
Possible Drawbacks
Verification Process
Release Notes
Contributing checklist
Summary by CodeRabbit
New Features
SHORT_ON_MIN_LEVELfor enhanced control of device activation levels.Bug Fixes
Documentation