You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Refactored TankAlarm client to support monitoring of tanks, engines, pumps, gas systems, and flow meters with flexible sensor interfaces (digital, analog, current loop, pulse). Updated configuration structures, runtime state, and documentation to enable multi-purpose monitoring and improved RPM/flow measurement for very low rates.
This review covers the "112025" version of the Tank Alarm system, specifically the Server, Client, and Viewer components designed for the Arduino Opta and Blues Wireless Notecard.
7
+
8
+
**Scope:**
9
+
-`TankAlarm-112025-Server-BluesOpta/`
10
+
-`TankAlarm-112025-Client-BluesOpta/`
11
+
-`TankAlarm-112025-Viewer-BluesOpta/`
12
+
13
+
## Summary
14
+
The codebase is well-structured, modular, and demonstrates a high level of maturity for an embedded system. It effectively leverages the capabilities of the Arduino Opta (STM32H7) and the Blues Notecard. The code handles complex tasks such as telemetry aggregation, remote configuration, and reliable communication with robust error handling.
15
+
16
+
## Key Strengths
17
+
1.**Architecture**: Clear separation of concerns between Server, Client, and Viewer roles. The Server acts as the central hub, aggregating data and managing configuration, while Clients are focused on sensing and reporting.
18
+
2.**Hardware Abstraction**: The code uses conditional compilation (`#ifdef`) effectively to support different hardware platforms (Opta/Mbed vs. STM32duino), ensuring portability.
19
+
3.**Robustness**:
20
+
-**Watchdog Timer**: Integrated watchdog support (`IWatchdog` or Mbed `Watchdog`) prevents system hangs.
21
+
-**Error Handling**: Notecard requests and JSON parsing are checked for errors.
22
+
-**Memory Management**: Heap allocation for large JSON documents is handled correctly with explicit `delete` calls.
23
+
4.**Configuration**: Extensive use of `#define` macros for compile-time configuration and `struct`s for runtime configuration.
24
+
5.**Features**:
25
+
-**Remote Configuration**: The system supports pushing configuration updates from the Server to Clients via the Notecard.
26
+
-**Calibration**: A learning-based calibration system is implemented.
27
+
-**Web Interface**: The Server hosts a comprehensive web dashboard and configuration tools.
28
+
29
+
## Findings & Recommendations
30
+
31
+
### 1. Network Configuration (High Priority)
32
+
**Observation:** IP addresses and MAC addresses are hardcoded in the firmware.
33
+
-`gStaticIp`, `gStaticGateway`, etc. are defined as `static` global variables.
34
+
-`gMacAddress` is hardcoded.
35
+
36
+
**Risk:** This limits deployment flexibility. Multiple devices on the same network would require recompilation to avoid IP/MAC conflicts.
37
+
38
+
**Recommendation:**
39
+
-**DHCP**: Enable DHCP by default, with a fallback to a static IP if DHCP fails.
40
+
-**Configuration File**: Load network settings (IP, Gateway, Subnet) from a configuration file on the filesystem (e.g., `network.json`) or from the Notecard's environment variables.
41
+
-**Unique MAC**: While the locally administered bit is used, ensure a mechanism to generate a unique MAC address (e.g., based on the MCU's unique ID) to avoid collisions.
42
+
43
+
### 2. String Usage (Medium Priority)
44
+
**Observation:** The `String` class is used in several places (e.g., `performFtpBackup`, HTTP handling).
45
+
46
+
**Risk:** On embedded systems, excessive use of `String` can lead to heap fragmentation over time, potentially causing instability.
47
+
48
+
**Recommendation:**
49
+
- While the Opta has ample RAM (1MB), it is best practice to minimize `String` usage. Prefer `char` arrays and `snprintf` for string manipulation where possible, especially in long-running loops.
50
+
51
+
### 3. Security (Medium Priority)
52
+
**Observation:**
53
+
- The system uses a PIN (`configPin`) for protecting sensitive web actions.
54
+
-`SERVER_PRODUCT_UID` and `VIEWER_PRODUCT_UID` are hardcoded.
55
+
56
+
**Recommendation:**
57
+
- Ensure the PIN is strong and changed from any default.
58
+
- Consider loading Product UIDs from a secure configuration or Notecard environment variables to allow for easier fleet management without recompilation.
59
+
60
+
### 4. Code Duplication (Low Priority)
61
+
**Observation:** There is some code duplication between the Server, Client, and Viewer sketches (e.g., `initializeNotecard`, `ensureTimeSync`, Watchdog setup).
62
+
63
+
**Recommendation:**
64
+
- If the project grows, consider creating a shared library (e.g., `TankAlarmCommon`) to encapsulate common functionality. This would reduce maintenance effort and ensure consistency.
65
+
66
+
### 5. Memory Safety (Medium Priority)
67
+
**Observation:**`DynamicJsonDocument` is allocated on the heap in multiple locations across Server, Client, and Viewer.
**Observation:** The FTP backup/restore implementation is functional but has several areas for improvement.
91
+
92
+
**Findings:**
93
+
94
+
#### A. Connection Management ✅
95
+
-`ftpQuit()` properly sends QUIT command and closes the control connection.
96
+
-`ftpConnectAndLogin()` validates host presence and connection success.
97
+
-`FtpSession` struct correctly encapsulates the control connection.
98
+
99
+
#### B. Error Handling ⚠️
100
+
-**Partial Failure Recovery**: When `performFtpBackup` fails mid-backup (e.g., after 2 of 5 files), there's no rollback or indication of which files succeeded.
101
+
-**Silent Failures**: In `ftpBackupClientConfigs`, individual file failures are logged but don't stop the backup or report to caller.
102
+
103
+
**Recommendation:** Consider returning a more detailed result structure:
104
+
```cpp
105
+
structFtpResult {
106
+
bool success;
107
+
uint8_t filesProcessed;
108
+
uint8_t filesFailed;
109
+
char lastError[128];
110
+
};
111
+
```
112
+
113
+
#### C. Buffer Size Concerns ⚠️
114
+
- **`ftpBackupClientConfigs`** uses a 2KB manifest buffer (line 1867) - adequate for ~40 clients with typical UID/site lengths.
115
+
- **`ftpRestoreClientConfigs`** uses a 1KB config buffer (line 1962) per client - may truncate large configs.
116
+
- **`FTP_MAX_FILE_BYTES` = 24KB** - enforced correctly in `writeBufferToFile` but not in all retrieve paths.
117
+
118
+
**Recommendation:** The 1KB client config buffer should be increased to match `FTP_MAX_FILE_BYTES` or at least 2KB to prevent silent truncation of larger configurations.
119
+
120
+
#### D. Timeout Handling ✅
121
+
- `FTP_TIMEOUT_MS` (8 seconds) is applied consistently in `ftpReadResponse` and `ftpRetrieveBuffer`.
122
+
- The implementation correctly handles slow/stalled connections.
123
+
124
+
#### E. Data Connection Cleanup ⚠️
125
+
- In `ftpStoreBuffer` and `ftpRetrieveBuffer`, the data connection (`dataClient`) is properly stopped before checking transfer completion.
126
+
- However, if `ftpEnterPassive` succeeds but `dataClient.connect` fails, subsequent operations may leave the server in an unexpected state.
127
+
128
+
**Recommendation:** After a data connection failure, consider sending `ABOR` to reset server state before the next operation.
129
+
130
+
#### F. Path Traversal Security ✅
131
+
- `buildRemotePath` constructs paths safely using `snprintf` with bounded output.
132
+
- No user-controlled input flows directly into path construction.
133
+
134
+
### 7. Client Memory Management (Low Priority)
135
+
**Observation:** The Client code (lines 929-946) uses `malloc`/`free` for file reading with proper null checks.
136
+
137
+
**Findings:**
138
+
- ✅ Null check after `malloc` (line 930 equivalent).
139
+
- ✅ `free(buffer)` called in both success (line 946) and error (line 941) paths.
140
+
- ✅ File size validated before allocation (8KB limit, line 924-927).
141
+
- ✅ `std::unique_ptr` used for `DynamicJsonDocument` in same function.
142
+
143
+
**Recommendation:** None - this pattern is correct and robust.
144
+
145
+
## Conclusion
146
+
The "112025" code release is in excellent shape. The logic is sound, and the implementation is robust. Addressing the network configuration rigidity is the most significant improvement to be made for scalable deployment.
Copy file name to clipboardExpand all lines: TankAlarm-112025-Client-BluesOpta/README.md
+74-17Lines changed: 74 additions & 17 deletions
Display the source diff
Display the rich diff
Original file line number
Diff line number
Diff line change
@@ -1,19 +1,43 @@
1
1
# TankAlarm 112025 Client - Blues Opta
2
2
3
-
Tank level monitoring client using Arduino Opta with Blues Wireless Notecard cellular connectivity.
3
+
Multi-purpose monitoring client using Arduino Opta with Blues Wireless Notecard cellular connectivity.
4
4
5
5
## Overview
6
6
7
-
The TankAlarm 112025 Client monitors tank levels using analog sensors and reports data to a central server via Blues Wireless Notecard. Key features include:
7
+
The TankAlarm 112025 Client monitors tanks, engines, pumps, and other equipment using various sensor types and reports data to a central server via Blues Wireless Notecard. Key features include:
8
8
9
9
-**Cellular connectivity** via Blues Wireless Notecard
10
-
-**Multi-tank support** - Monitor up to 8 tanks per device
11
-
-**Configurable alarms** - High and low thresholds per tank
10
+
-**Multi-monitor support** - Monitor up to 8 sensors per device
- With sampleSeconds=1800 (30 min) and 1 pulse/rev: detects down to 0.033 RPM
289
+
255
290
**Configuration Parameters:**
256
291
-`sensorType`: "rpm"
257
292
-`rpmPin`: Digital input pin for hall effect sensor (uses internal pull-up)
@@ -261,6 +296,12 @@ Hall effect sensors can be used to measure RPM (rotations per minute) for applic
261
296
-**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.
262
297
-`hallEffectType`: Sensor type - "unipolar", "bipolar", "omnipolar", or "analog"
263
298
-`hallEffectDetection`: Detection method - "pulse" or "time"
299
+
-`rpmSampleDurationMs`: Sample duration in milliseconds (default: 60000 = 60 seconds)
300
+
- Longer durations detect lower RPM but increase measurement time
301
+
- For 0.1 RPM detection without accumulated mode: use 600000 (10 minutes)
302
+
-`rpmAccumulatedMode`: Set to `true` for very low RPM measurement (< 1 RPM)
303
+
- Counts pulses between telemetry reports instead of during a fixed sample window
304
+
- Best for applications where RPM is very slow (e.g., 0.1 RPM = 1 rotation per 10 minutes)
264
305
-`highAlarm`: Maximum expected RPM for alarm (e.g., 3000)
265
306
-`lowAlarm`: Minimum expected RPM for alarm (e.g., 100)
266
307
@@ -278,6 +319,21 @@ Hall effect sensors can be used to measure RPM (rotations per minute) for applic
278
319
```
279
320
Note: With omnipolar sensor and 4 magnets, use pulsesPerRev = 8 (2 pulses per magnet × 4 magnets)
280
321
322
+
**Example Configuration** (Slow pump flow meter - 0.1 RPM detection):
323
+
```json
324
+
{
325
+
"sensor": "rpm",
326
+
"rpmPin": 3,
327
+
"pulsesPerRev": 1,
328
+
"hallEffectType": "unipolar",
329
+
"hallEffectDetection": "pulse",
330
+
"rpmAccumulatedMode": true,
331
+
"highAlarm": 10,
332
+
"lowAlarm": 0.05
333
+
}
334
+
```
335
+
Note: With accumulated mode enabled and 30-minute sample intervals, this can detect down to 0.033 RPM
336
+
281
337
**Wiring:**
282
338
- Connect hall effect sensor VCC to 5V or 3.3V (check sensor datasheet)
283
339
- Connect sensor GND to Arduino GND
@@ -287,6 +343,7 @@ Note: With omnipolar sensor and 4 magnets, use pulsesPerRev = 8 (2 pulses per ma
287
343
**Tips:**
288
344
- Use "time" detection for faster response to speed changes
289
345
- Use "pulse" detection for better accuracy at steady speeds
346
+
- Use `rpmAccumulatedMode: true` for very slow rotation (< 1 RPM)
290
347
- For multiple magnets, set `pulsesPerRev` to the number of magnets
291
348
- For omnipolar sensors, each magnet passing creates 2 pulses (both N and S poles trigger)
292
349
- Monitor both high and low thresholds to detect over-speed and stall conditions
0 commit comments