Skip to content

Commit 3a1efeb

Browse files
Copilotdorkmo
andcommitted
Address code review feedback: add default cases, fix docs, consolidate constants
Co-authored-by: dorkmo <[email protected]>
1 parent ed8fef8 commit 3a1efeb

File tree

2 files changed

+18
-6
lines changed

2 files changed

+18
-6
lines changed

TankAlarm-112025-Client-BluesOpta/README.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -258,6 +258,7 @@ Hall effect sensors can be used to measure RPM (rotations per minute) for applic
258258
- `pulsesPerRevolution`: Number of pulses generated per complete rotation (default: 1)
259259
- Single magnet: 1 pulse per revolution
260260
- Multiple magnets: Set to number of magnets
261+
- **Note:** For **bipolar** or **omnipolar** sensors, a single magnet generates 2 pulses per revolution (one for each pole). Set `pulsesPerRevolution` to `2 × number of magnets` in these cases.
261262
- `hallEffectType`: Sensor type - "unipolar", "bipolar", "omnipolar", or "analog"
262263
- `hallEffectDetection`: Detection method - "pulse" or "time"
263264
- `highAlarm`: Maximum expected RPM for alarm (e.g., 3000)
@@ -286,7 +287,7 @@ Hall effect sensors can be used to measure RPM (rotations per minute) for applic
286287
- Use "time" detection for faster response to speed changes
287288
- Use "pulse" detection for better accuracy at steady speeds
288289
- For multiple magnets, set `pulsesPerRev` to the number of magnets
289-
- For omnipolar sensors, each magnet passing creates a pulse (N and S poles both trigger)
290+
- For omnipolar sensors, each magnet passing creates 2 pulses (both N and S poles trigger)
290291
- Monitor both high and low thresholds to detect over-speed and stall conditions
291292

292293
## Operation

TankAlarm-112025-Client-BluesOpta/TankAlarm-112025-Client-BluesOpta.ino

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -873,13 +873,17 @@ static bool loadConfigFromFlash(ClientConfig &cfg) {
873873
cfg.tanks[i].hallEffectType = HALL_EFFECT_OMNIPOLAR;
874874
} else if (hallType && strcmp(hallType, "analog") == 0) {
875875
cfg.tanks[i].hallEffectType = HALL_EFFECT_ANALOG;
876+
} else if (hallType && strcmp(hallType, "unipolar") == 0) {
877+
cfg.tanks[i].hallEffectType = HALL_EFFECT_UNIPOLAR;
876878
} else {
877879
cfg.tanks[i].hallEffectType = HALL_EFFECT_UNIPOLAR; // Default
878880
}
879881
// Load hall effect detection method
880882
const char *hallDetect = t["hallEffectDetection"].as<const char *>();
881883
if (hallDetect && strcmp(hallDetect, "time") == 0) {
882884
cfg.tanks[i].hallEffectDetection = HALL_DETECT_TIME_BASED;
885+
} else if (hallDetect && strcmp(hallDetect, "pulse") == 0) {
886+
cfg.tanks[i].hallEffectDetection = HALL_DETECT_PULSE;
883887
} else {
884888
cfg.tanks[i].hallEffectDetection = HALL_DETECT_PULSE; // Default
885889
}
@@ -1894,6 +1898,10 @@ static float readTankSensor(uint8_t idx) {
18941898
uint8_t pulsesPerRev = (cfg.pulsesPerRevolution > 0) ? cfg.pulsesPerRevolution : 1;
18951899
const float MS_PER_MINUTE = 60000.0f;
18961900

1901+
// Common constants for both detection methods
1902+
const unsigned long DEBOUNCE_MS = 2;
1903+
const uint32_t MAX_ITERATIONS = RPM_SAMPLE_DURATION_MS * 2;
1904+
18971905
float rpm = 0.0f;
18981906

18991907
// Choose detection method: pulse counting or time-based
@@ -1905,9 +1913,6 @@ static float readTankSensor(uint8_t idx) {
19051913
unsigned long sampleStart = millis();
19061914
int lastState = digitalRead(pin);
19071915
gRpmLastPinState[idx] = lastState;
1908-
1909-
const unsigned long DEBOUNCE_MS = 2;
1910-
const uint32_t MAX_ITERATIONS = RPM_SAMPLE_DURATION_MS * 2;
19111916
unsigned long firstPulseTime = 0;
19121917
unsigned long secondPulseTime = 0;
19131918
unsigned long cycleLastPulseTime = 0; // Track last pulse within this measurement cycle for debounce
@@ -1932,6 +1937,10 @@ static float readTankSensor(uint8_t idx) {
19321937
// Bipolar/Latching and Omnipolar: detect both edges (state changes)
19331938
edgeDetected = (lastState != currentState);
19341939
break;
1940+
default:
1941+
// Default to unipolar behavior if value is invalid/corrupted
1942+
edgeDetected = (lastState == HIGH && currentState == LOW);
1943+
break;
19351944
}
19361945

19371946
if (edgeDetected) {
@@ -1997,8 +2006,6 @@ static float readTankSensor(uint8_t idx) {
19972006
int lastState = digitalRead(pin);
19982007
gRpmLastPinState[idx] = lastState;
19992008

2000-
const unsigned long DEBOUNCE_MS = 2;
2001-
const uint32_t MAX_ITERATIONS = RPM_SAMPLE_DURATION_MS * 2;
20022009
unsigned long lastPulseTime = 0;
20032010
uint32_t iterationCount = 0;
20042011

@@ -2018,6 +2025,10 @@ static float readTankSensor(uint8_t idx) {
20182025
// Bipolar/Latching and Omnipolar: count both edges (state changes)
20192026
edgeDetected = (lastState != currentState);
20202027
break;
2028+
default:
2029+
// Default to unipolar behavior for safety and consistency
2030+
edgeDetected = (lastState == HIGH && currentState == LOW);
2031+
break;
20212032
}
20222033

20232034
if (edgeDetected) {

0 commit comments

Comments
 (0)