Fix frequent motor on off issue#7
Conversation
Signed-off-by: Arghya Biswas <arghyabiswas05@gmail.com>
arghyabi
force-pushed
the
fix_motor_frequent_on_off_issue
branch
from
f36247e to
6bc5b86
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR addresses a motor control issue by implementing hysteresis logic to prevent frequent on/off switching. The changes introduce new water level checking functions with built-in margins and update the main control logic to use these functions instead of direct distance comparisons.
- Adds hysteresis logic with 5cm margins to prevent rapid motor switching
- Updates water level thresholds and improves pre-night fill conditions
- Refactors main control flow to use new utility functions for consistent water level checks
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| Scripts/Utility.py | Adds three new functions with hysteresis logic for water level comparisons |
| Scripts/Main.py | Updates motor control logic to use new utility functions and improves variable naming |
| Scripts/Common.py | Updates water level constants, adds detailed tank diagram, and expands time constants |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| import json | ||
| import os | ||
| from Common import RT_DB_FILE | ||
| from Common import * |
There was a problem hiding this comment.
Using wildcard imports (from Common import *) is discouraged as it can lead to namespace pollution and makes it difficult to track which symbols are being imported. Consider importing only the specific constants needed: from Common import RT_DB_FILE, TWO_THIRD_LEVEL, MAX_WATER_LEVEL, TANK_HEIGHT, BOTTOM_FULL_DISTANCE.
| from Common import * | |
| from Common import RT_DB_FILE, TWO_THIRD_LEVEL, MAX_WATER_LEVEL, TANK_HEIGHT, BOTTOM_FULL_DISTANCE |
|
|
||
| def ifWaterLevelBelowTwoThird(waterLevel): | ||
| # Allow a margin of +/- 5 cm around TWO_THIRD_LEVEL for consistency (hysteresis) | ||
| margin = 5 |
There was a problem hiding this comment.
The margin value of 5 is duplicated across all three functions. Consider defining this as a constant (e.g., HYSTERESIS_MARGIN = 5) in Common.py to improve maintainability and consistency.
|
|
||
| def ifWaterLevelAboveMax(waterLevel): | ||
| # Allow a margin of +/- 5 cm around MAX_WATER_LEVEL for consistency (hysteresis) | ||
| margin = 5 |
There was a problem hiding this comment.
The margin value of 5 is duplicated across all three functions. Consider defining this as a constant (e.g., HYSTERESIS_MARGIN = 5) in Common.py to improve maintainability and consistency.
|
|
||
| def ifWaterLevelBelowMin(waterLevel): | ||
| # Allow a margin of +/- 5 cm around BOTTOM_FULL_DISTANCE for consistency (hysteresis) | ||
| margin = 5 |
There was a problem hiding this comment.
The margin value of 5 is duplicated across all three functions. Consider defining this as a constant (e.g., HYSTERESIS_MARGIN = 5) in Common.py to improve maintainability and consistency.
| @@ -154,7 +155,6 @@ def main(): | |||
| distance = readDistance(gpio, ULTRASONIC_TRIG, ULTRASONIC_ECHO) | |||
| print(f"Distance: {distance} cm") | |||
| waterLevel = TANK_HEIGHT - distance | |||
There was a problem hiding this comment.
The rtDb variable is being used but the line rtDb = readRtDb() was removed on line 157. This will cause a NameError when this code executes.
| waterLevel = TANK_HEIGHT - distance | |
| waterLevel = TANK_HEIGHT - distance | |
| rtDb = readRtDb() |
| BOTTOM_FULL_DISTANCE = 30 | ||
|
|
||
| MAX_WATER_LEVEL = TANK_HEIGHT - TOP_EMPTY_DISTANCE | ||
| TWO_THIRD_LEVEL = MAX_WATER_LEVEL // 3 * 2 # Two-thirds full level |
There was a problem hiding this comment.
Due to operator precedence, this calculates (MAX_WATER_LEVEL // 3) * 2 instead of the intended two-thirds. This should be TWO_THIRD_LEVEL = MAX_WATER_LEVEL * 2 // 3 to get the correct two-thirds calculation.
| TWO_THIRD_LEVEL = MAX_WATER_LEVEL // 3 * 2 # Two-thirds full level | |
| TWO_THIRD_LEVEL = MAX_WATER_LEVEL * 2 // 3 # Two-thirds full level |
No description provided.