Add new manual mode and fix initial distance read issue#9
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds a new manual mode feature that allows users to switch between Auto and Manual operation modes, and fixes an initial distance reading issue in the main controller.
- Introduces a Manual/Auto mode selector in the web interface
- Fixes initial distance sensor reading with proper validation loop
- Updates default mode handling and motor control logic based on selected mode
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| config.yaml | Updates application version to 1.3.0.1007 |
| Web/style.css | Adds styling for mode selection UI components |
| Web/script.js | Implements mode selection functionality and motor button state management |
| Web/motor.php | Adds mode configuration support to backend API |
| Web/index.html | Adds mode selection dropdown to the interface |
| Scripts/Utility.py | Extends database utility to support mode parameter |
| Scripts/Main.py | Implements mode-based motor control logic and fixes initial distance reading |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| "tankLevel" => isset($data['tankLevel']) ? $data['tankLevel'] : 0, | ||
| "valve1Duration" => isset($data['valve1Duration']) ? $data['valve1Duration'] : 1, | ||
| "valve2Duration" => isset($data['valve2Duration']) ? $data['valve2Duration'] : 1, | ||
| "mode" => isset($data['mode']) ? $data['mode'] : 'Manual', |
There was a problem hiding this comment.
Default mode inconsistency: the web interface defaults to 'Manual' but the Python script defaults to 'Auto' (line 73 in Main.py and line 157). This could cause confusion when the system starts up.
| "mode" => isset($data['mode']) ? $data['mode'] : 'Manual', | |
| "mode" => isset($data['mode']) ? $data['mode'] : 'Auto', |
| print("Tank level OK: Motor OFF") | ||
| print("Auto mode - Tank level OK: Motor OFF") | ||
| else: | ||
| print(f"Unknown mode '{currentMode}', defaulting to Auto mode behavior") |
There was a problem hiding this comment.
The system logs that it's defaulting to Auto mode behavior for unknown modes, but it doesn't actually change the mode value or update the database. This could lead to inconsistent state where the UI shows an invalid mode while the system behaves as Auto.
| print(f"Unknown mode '{currentMode}', defaulting to Auto mode behavior") | |
| print(f"Unknown mode '{currentMode}', defaulting to Auto mode behavior") | |
| currentMode = "Auto" | |
| writeRtDb(mode="Auto") |
arghyabi
force-pushed
the
fix_initial_reading
branch
from
835286c to
05b3bb1
Compare
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| data.motorStatus === 'ON' ? 'ON' : 'OFF'; | ||
|
|
||
| // Update mode display | ||
| const mode = data.mode || 'Manual'; |
There was a problem hiding this comment.
Default mode mismatch: JavaScript defaults to 'Manual' but PHP's getStatus() function defaults to 'Auto'. This inconsistency could cause unexpected behavior when no mode is set in the database.
| const mode = data.mode || 'Manual'; | |
| const mode = data.mode || 'Auto'; |
| print("Auto mode - Tank level OK: Motor OFF") | ||
| else: | ||
| print(f"Unknown mode '{currentMode}', defaulting to Auto mode behavior") | ||
| writeRtDb(mode = "Auto") |
There was a problem hiding this comment.
Using extra spaces around the equals sign in the function call writeRtDb(mode = \"Auto\") is inconsistent with Python PEP 8 style guidelines. Should be writeRtDb(mode=\"Auto\").
| writeRtDb(mode = "Auto") | |
| writeRtDb(mode="Auto") |
Signed-off-by: Arghya Biswas <arghyabiswas05@gmail.com>
arghyabi
force-pushed
the
fix_initial_reading
branch
from
05b3bb1 to
0f38721
Compare
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| $mode = $_POST['mode'] === 'Auto' ? 'Auto' : 'Manual'; | ||
| echo json_encode(setConfig('mode', $mode)); |
There was a problem hiding this comment.
The mode validation only accepts 'Auto' or defaults to 'Manual' for any other value. This could allow unexpected values to be set as 'Manual'. Consider explicit validation that only accepts 'Auto' or 'Manual' and returns an error for invalid values.
| $mode = $_POST['mode'] === 'Auto' ? 'Auto' : 'Manual'; | |
| echo json_encode(setConfig('mode', $mode)); | |
| $mode = $_POST['mode']; | |
| if ($mode === 'Auto' || $mode === 'Manual') { | |
| echo json_encode(setConfig('mode', $mode)); | |
| } else { | |
| echo json_encode(['error' => "Invalid mode value. Only 'Auto' or 'Manual' are allowed."]); | |
| } |
No description provided.