Remove obsolete MQTT documentation files, including buffer optimization, library alternatives, memory analysis, responsiveness optimizations, and stability review. These files are no longer relevant to the current implementation and have been superseded by updated documentation.

This commit is contained in:
agessaman committed 2026-03-28 17:17:03 -07:00
1 parent ff49d28ffb
commit 384b80fc05
7 files changed
+3 -7464

No files matched your search

-113
View File
@@ -1,113 +0,0 @@
# MQTT Buffer Optimization Using getMqttConfig()
## Summary
We've successfully implemented memory optimizations for all MQTT clients using `PsychicMqttClient::getMqttConfig()` to access the underlying ESP-IDF MQTT client configuration.
## Implementation
### New Function: `optimizeMqttClientConfig()`
Added a private helper function that optimizes MQTT client buffer sizes:
```cpp
void MQTTBridge::optimizeMqttClientConfig(PsychicMqttClient* client);
```
### Optimizations Applied
1. **Reduced Buffer Size**: Changed from default 1024 bytes to 512 bytes
- Applied via `client->setBufferSize(512)`
- Saves ~512 bytes per MQTT client
- Our JSON messages are typically <500 bytes, so 512 is sufficient
2. **ESP-IDF v5 Output Buffer**: Reduced `buffer.out_size` to 512 bytes
- Applied via direct config access: `config->buffer.out_size = 512`
- Saves an additional ~512 bytes per client on ESP-IDF v5
### Memory Savings
**Per Client Savings:**
- Input buffer: 1024 → 512 bytes = **512 bytes saved**
- Output buffer (ESP-IDF v5): 1024 → 512 bytes = **512 bytes saved**
- **Total per client: ~1KB saved** (ESP-IDF v5) or **512 bytes** (ESP-IDF v4)
**Total System Savings:**
- 3 MQTT clients (main + US analyzer + EU analyzer)
- ESP-IDF v5: **~3KB total savings**
- ESP-IDF v4: **~1.5KB total savings**
### Applied To
The optimization is automatically applied to all three MQTT clients:
1. `_mqtt_client` - Main MQTT client (in `begin()`)
2. `_analyzer_us_client` - US analyzer server client (in `setupAnalyzerClients()`)
3. `_analyzer_eu_client` - EU analyzer server client (in `setupAnalyzerClients()`)
## Code Changes
### Files Modified
1. **`src/helpers/bridges/MQTTBridge.h`**
- Added `optimizeMqttClientConfig()` declaration
2. **`src/helpers/bridges/MQTTBridge.cpp`**
- Added `optimizeMqttClientConfig()` implementation
- Called after creating each MQTT client instance
## Technical Details
### ESP-IDF Version Detection
The code handles both ESP-IDF v4 and v5:
- **ESP-IDF v5**: Uses `config->buffer.size` and `config->buffer.out_size`
- **ESP-IDF v4**: Uses `config->buffer_size` (set via `setBufferSize()`)
### Buffer Size Rationale
- **Default**: 1024 bytes (ESP-IDF default)
- **Optimized**: 512 bytes
- **Justification**:
- Status messages: ~400-500 bytes
- Packet messages: ~400-1500 bytes (most <500)
- Raw messages: ~200-400 bytes
- 512 bytes is sufficient for 95%+ of messages
- Larger messages will be fragmented (handled automatically by ESP-IDF)
## Impact
### Memory Benefits
- **Reduced per-client memory footprint**
- **Lower heap fragmentation** (smaller allocations)
- **More headroom for other operations**
### Potential Trade-offs
- **Message fragmentation**: Messages >512 bytes will be split into multiple chunks
- ESP-IDF handles this automatically
- No functional impact, just slightly more overhead for large messages
- **Large packet responses**: Neighbors list responses (~1500 bytes) will be fragmented
- This is acceptable as they're infrequent
## Testing Recommendations
1. **Monitor memory usage**: Check if Max alloc improves
2. **Verify functionality**: Ensure all MQTT publishes still work correctly
3. **Check fragmentation**: Monitor if large messages are handled properly
4. **Long-term stability**: Run for extended periods to verify no regressions
## Related Optimizations
This complements existing memory optimizations:
- ✅ Memory pressure monitoring (skip publishes when Max alloc < 60KB)
- ✅ Reduced raw data storage for TX packets
- ✅ Consolidated analyzer server publishing
- ✅ Buffer size optimization (this change)
## Future Considerations
If memory pressure persists, consider:
1. Further reducing buffer size to 256 bytes (may cause more fragmentation)
2. Using synchronous publishes (`async=false`) to reduce queue overhead
3. Reducing to single analyzer server (eliminates one client entirely)
4. Custom MQTT implementation with zero-copy design (significant effort)
+3 -3
View File
@@ -15,10 +15,10 @@ Use one of the observer build targets (e.g., `heltec_v4_repeater_observer_mqtt`)
If this is a fresh flash or full erase, configure your radio parameters first. These must match other nodes in your mesh:
```bash
set freq 906.0
set sf 12
set freq 910.525
set sf 7
set cr 5
set bw 250
set bw 62.5
set tx 22
```
-155
View File
@@ -1,155 +0,0 @@
# MQTT Library Alternatives with WebSocket Support
## Current Library
- **PsychicMqttClient** (`elims/PsychicMqttClient@^0.2.4`)
- **WebSocket Support**: ✅ Yes (WSS://)
- **Memory Issue**: ESP-IDF `esp_mqtt_client_enqueue()` copies payloads internally
- **Platform**: ESP32 only
## Alternative Libraries with WebSocket Support
### 1. AsyncMqttClient ⭐ **RECOMMENDED**
- **Author**: Marvin Roger
- **GitHub**: https://github.com/marvinroger/AsyncMqttClient
- **PlatformIO**: `marvinroger/AsyncMqttClient`
- **WebSocket Support**: ✅ Yes (WSS://)
- **Memory Management**:
- Uses ESP-IDF MQTT client (same underlying library as PsychicMqttClient)
- **Same memory issue**: ESP-IDF copies payloads internally
- **Pros**:
- Well-established, widely used
- Similar API to PsychicMqttClient (almost drop-in replacement)
- Good documentation
- **Cons**:
- Same underlying ESP-IDF library = same memory fragmentation issue
- May not solve the memory problem
### 2. ESP-IDF MQTT Client (Direct Usage)
- **Library**: Built into ESP-IDF framework
- **WebSocket Support**: ✅ Yes (WSS://)
- **Memory Management**:
- Same as PsychicMqttClient (it's a wrapper)
- Can configure buffer sizes via `esp_mqtt_client_config_t`
- May allow more control over memory
- **Pros**:
- No wrapper overhead
- Direct control over configuration
- Can set buffer sizes, queue depths
- **Cons**:
- More complex API
- Requires ESP-IDF knowledge
- Still copies payloads internally
- **Configuration Options**:
```cpp
esp_mqtt_client_config_t mqtt_cfg = {
.buffer.size = 1024, // Can reduce this
.buffer.out_size = 1024, // Can reduce this
// ... other config
};
```
### 3. lwmqtt (Lightweight MQTT)
- **GitHub**: https://github.com/256dpi/lwmqtt
- **WebSocket Support**: ❌ **NO** - TCP only
- **Memory Management**:
- Zero-copy design
- No dynamic allocations
- Fixed buffers
- **Pros**:
- Excellent memory efficiency
- Zero-copy, no fragmentation
- Very lightweight
- **Cons**:
- **No WebSocket support** (deal breaker for analyzer servers)
- Would need separate WebSocket implementation
- More complex integration
### 4. PubSubClient
- **PlatformIO**: `knolleary/PubSubClient`
- **WebSocket Support**: ❌ **NO** - TCP only
- **Memory Management**:
- Uses fixed buffer (configurable size)
- Copies payloads into buffer
- **Pros**:
- Simple API
- Widely used
- Predictable memory usage
- **Cons**:
- **No WebSocket support** (deal breaker)
- Synchronous (blocks)
- Less efficient than async libraries
### 5. Custom WebSocket + MQTT Implementation
- **Approach**: Use ESP-IDF WebSocket client + custom MQTT protocol layer
- **WebSocket Support**: ✅ Yes (full control)
- **Memory Management**:
- Full control over allocations
- Can implement zero-copy
- Custom buffer management
- **Pros**:
- Complete control over memory
- Can optimize for our use case
- No unnecessary copies
- **Cons**:
- Significant development effort
- Need to implement MQTT protocol
- Testing and maintenance burden
## Recommendation
### Option A: Stay with PsychicMqttClient + Optimize Usage ⭐ **BEST SHORT-TERM**
- **Why**: All ESP32 MQTT libraries use ESP-IDF underneath = same memory issue
- **Actions**:
1. Reduce number of publishes (single analyzer server)
2. Test synchronous publishes (`async=false`)
3. Configure ESP-IDF buffer sizes via PsychicMqttClient
4. Keep memory pressure monitoring
### Option B: Switch to AsyncMqttClient
- **Why**: More mature, better documented, similar API
- **Trade-off**: Same memory issue (uses ESP-IDF)
- **Effort**: Medium (API is similar, mostly drop-in)
### Option C: Use ESP-IDF MQTT Client Directly
- **Why**: More control over configuration
- **Actions**:
- Bypass wrapper library
- Configure buffer sizes directly
- May reduce some overhead
- **Effort**: High (need to rewrite MQTT bridge code)
- **Benefit**: Can tune ESP-IDF buffer sizes
### Option D: Custom Implementation (Long-term)
- **Why**: Complete control over memory
- **Effort**: Very High (weeks of development)
- **Benefit**: Optimal memory usage, zero-copy possible
## Key Finding
⚠️ **All ESP32 MQTT libraries that support WebSockets use ESP-IDF `esp_mqtt_client` underneath**, which copies payloads internally. This is a limitation of the ESP-IDF framework, not the wrapper libraries.
**Options to reduce memory impact:**
1. ✅ Reduce number of publishes (already identified)
2. ✅ Memory pressure monitoring (already implemented)
3. ⚠️ Configure ESP-IDF buffer sizes (may help)
4. ⚠️ Use synchronous publishes (may reduce queue overhead)
5. ⚠️ Custom implementation (significant effort)
## Next Steps
1. **Test ESP-IDF Buffer Configuration**:
- Try reducing `setBufferSize()` in PsychicMqttClient
- May reduce per-client memory but won't fix publish allocations
2. **Test Synchronous Publishes**:
- Try `async=false` to bypass queue
- May reduce memory but blocks execution
3. **Reduce Publishes**:
- Implement single analyzer server
- Measure memory improvement
4. **Consider ESP-IDF Direct Usage**:
- If other optimizations don't help enough
- More control but more complexity
-208
View File
@@ -1,208 +0,0 @@
# MQTT Library Memory Analysis
## Library Information
- **Library**: `elims/PsychicMqttClient@^0.2.4`
- **Type**: External PlatformIO library (wrapper around ESP-IDF MQTT client)
- **Platform**: ESP32
- **Protocol**: MQTT over WebSocket (WSS) for analyzer servers, standard MQTT for custom brokers
- **Underlying Library**: ESP-IDF `esp_mqtt_client` (part of ESP-IDF framework)
- **Source Location**: `.pio/libdeps/*/PsychicMqttClient/src/`
## Current Usage Patterns
### Memory Allocation Points
1. **Client Instance Creation**
```cpp
_mqtt_client = new PsychicMqttClient(); // Heap allocation
_analyzer_us_client = new PsychicMqttClient(); // Heap allocation
_analyzer_eu_client = new PsychicMqttClient(); // Heap allocation
```
- **Impact**: 3 instances × ~few KB each = ~10-15KB base memory
2. **setServer() Calls** ✅ **OPTIMIZED - NO ALLOCATION IN WRAPPER**
```cpp
// From PsychicMqttClient.cpp line 215-223
PsychicMqttClient &setServer(const char *uri) {
_mqtt_cfg.broker.address.uri = uri; // Just stores pointer
return *this;
}
```
- **Finding**: Wrapper does NOT copy URI (just stores pointer)
- **Current Optimization**: We only call `setServer()` when URI changes (using static tracking) ✅
- **ESP-IDF Behavior**: When `connect()` is called, ESP-IDF likely copies the URI internally
- **Impact**: Minimal (only on connection, not per publish)
3. **setCredentials() Calls** ✅ **NO ALLOCATION IN WRAPPER**
```cpp
// From PsychicMqttClient.cpp line 166-178
PsychicMqttClient &setCredentials(const char *username, const char *password) {
_mqtt_cfg.credentials.username = username; // Just stores pointers
_mqtt_cfg.credentials.authentication.password = password;
return *this;
}
```
- **Finding**: Wrapper does NOT copy credentials (just stores pointers)
- **Frequency**: Once per broker connection
- **ESP-IDF Behavior**: Likely copies when connecting
- **Impact**: Minimal (only on connection, not per publish)
4. **publish() Calls** ⚠️ **CONFIRMED MAIN CULPRIT**
```cpp
// From PsychicMqttClient.cpp line 370-389
int publish(const char *topic, int qos, bool retain, const char *payload, int length, bool async)
{
if (async) {
return esp_mqtt_client_enqueue(_client, topic, payload, length, qos, retain, true);
} else {
return esp_mqtt_client_publish(_client, topic, payload, length, qos, retain);
}
}
```
- **Frequency**: High (every packet, status updates)
- **Parameters**:
- `topic`: String pointer (passed to ESP-IDF)
- `payload`: Buffer pointer + length (passed to ESP-IDF)
- **Critical Finding**:
- PsychicMqttClient wrapper does NOT copy payload (passes pointers directly)
- **BUT**: ESP-IDF `esp_mqtt_client_enqueue()` and `esp_mqtt_client_publish()` **DO copy internally**
- ESP-IDF copies both topic and payload into internal buffers for async processing
- **Memory Impact**:
- Each publish: `strlen(topic)` + `payload_len` bytes allocated by ESP-IDF
- For 2KB JSON payloads + ~50 byte topics: ~2KB per publish
- **Multiple Publishes Per Packet**:
- Custom brokers: 1-3 publishes (one per broker)
- Analyzer servers: 2 publishes (US + EU)
- **Total**: Up to 5 publishes per packet × 2KB = **10KB+ allocations per packet**
- **Async vs Sync**:
- We use `async=true` (default), which calls `esp_mqtt_client_enqueue()`
- Enqueue likely allocates more (needs queue buffer) than direct publish
## Memory Fragmentation Evidence
### Observed Behavior
- **When MQTT Active**: Max alloc drops to ~54KB (severe fragmentation)
- **When MQTT Disconnected**: Max alloc recovers to ~88KB
- **Pattern**: Memory recovers when publishes stop
### Root Cause - CONFIRMED ✅
1. **ESP-IDF `esp_mqtt_client_enqueue()` copies payload internally**:
- PsychicMqttClient wrapper passes pointers (no copy in wrapper)
- ESP-IDF MQTT client copies topic + payload for async queue processing
- Each publish: `topic_len + payload_len` bytes allocated by ESP-IDF
- **This is the main source of fragmentation**
2. **Multiple publishes per packet** multiply allocations:
- 1 packet → 5 publishes (3 brokers + 2 analyzers) = 5× allocations
- Each allocation: ~2KB (topic + payload)
- **Total: ~10KB per packet in heap allocations**
3. **Frequent publishes** prevent heap coalescing:
- New allocations before old ones are freed
- ESP-IDF async queue holds messages until sent
- Creates "holes" in heap that can't be coalesced
## Recommendations
### 1. Investigate Library Source Code
Since the library is external, we need to:
- Check if library source is available in PlatformIO cache
- Review `publish()` implementation for memory allocations
- Look for zero-copy or buffer reuse options
### 2. Potential Optimizations
#### A. Reduce Number of Publishes ⭐ **HIGHEST IMPACT**
- **Current**: Publish to all brokers + analyzers separately (5 publishes per packet)
- **Option 1**: Publish to one analyzer server only (reduce from 2 to 1)
- **Impact**: 20% reduction in allocations (2KB saved per packet)
- **Option 2**: Use single custom broker with forwarding
- **Impact**: 60% reduction (from 3 brokers to 1)
- **Option 3**: Combine both (1 broker + 1 analyzer)
- **Impact**: 60% reduction (from 5 to 2 publishes)
- **Trade-off**: Less redundancy, simpler architecture
#### B. Reduce Payload Sizes
- **Current**: Up to 2KB JSON per packet
- **Option**: Compress or reduce JSON size
- **Impact**: Reduces allocation size per publish
- **Trade-off**: Less data per message
#### C. Throttle Publishes (Already Implemented) ✅
- **Current**: Skip publishes when Max alloc < 60KB
- **Status**: ✅ Implemented
- **Effect**: Prevents further fragmentation when memory is low
#### D. Use Synchronous Publishes (Blocking)
- **Current**: `async=true` (default) uses `esp_mqtt_client_enqueue()`
- **Option**: Use `async=false` to call `esp_mqtt_client_publish()` directly
- **Impact**: May reduce queue overhead, but blocks until sent
- **Trade-off**: Blocks execution, may impact responsiveness
- **Code Change**: `publish(topic, qos, retain, payload, len, false)`
#### E. Reduce Buffer Size Configuration
- **Current**: Default buffer size is 1024 bytes (from `setBufferSize()`)
- **Option**: Reduce buffer size if messages are smaller
- **Impact**: Less memory per client instance
- **Note**: May cause message fragmentation if payloads exceed buffer
#### F. Investigate ESP-IDF MQTT Client Configuration
- ESP-IDF MQTT client has internal buffer configuration
- May be able to reduce queue depth or buffer sizes
- **Requires**: ESP-IDF documentation review
### 3. Library Alternatives (If Issues Persist)
Consider lightweight MQTT libraries with better memory management:
- **lwmqtt**: Zero-copy, no dynamic allocations
- **coreMQTT**: Fixed buffers, predictable memory
- **Custom implementation**: Full control over memory
## Next Steps
1. ✅ **Locate Library Source**: Found in `.pio/libdeps/*/PsychicMqttClient/src/`
2. ✅ **Analyze `publish()` Implementation**:
- Wrapper doesn't copy (passes pointers)
- ESP-IDF `esp_mqtt_client_enqueue()` copies internally
- This is the root cause
3. **Test Synchronous Publishes**:
- Try `async=false` to use `esp_mqtt_client_publish()` directly
- May have different memory behavior (no queue)
- Measure fragmentation impact
4. **Reduce Number of Publishes**:
- Implement single analyzer server (US or EU)
- Or reduce custom broker count
- Measure memory improvement
5. **Profile ESP-IDF MQTT Client**:
- Check ESP-IDF documentation for buffer configuration
- Look for queue depth settings
- Investigate if we can reduce internal buffers
6. **Consider ESP-IDF MQTT Client Direct Usage**:
- Bypass PsychicMqttClient wrapper
- Use ESP-IDF API directly with custom memory management
- More control, but more complex
7. **Alternative: Switch to Different Library**:
- Consider `lwmqtt` (zero-copy, no dynamic allocations)
- Or `coreMQTT` (fixed buffers)
- Requires significant refactoring
## Current Mitigations
✅ **Memory Pressure Monitoring**: Skip publishes when fragmented
✅ **setServer() Optimization**: Only call when URI changes
✅ **JSON Buffer Reuse**: Build once, publish to multiple destinations
✅ **Analyzer Server Throttling**: Skip when memory low
## Remaining Risk
⚠️ **Library Internal Allocations**: If `publish()` copies payloads internally, we can't control this without:
- Library modification
- Library replacement
- Library API changes (if supported)
-142
View File
@@ -1,142 +0,0 @@
# MQTT Implementation Responsiveness Review & Optimizations
## Summary
This document outlines the responsiveness issues identified in the MQTT implementation and the optimizations applied to ensure repeater functions remain responsive.
## Issues Identified
### 1. **Blocking Operations**
- **Issue**: `delay(500)` calls in `begin()` blocked the entire system during WiFi connection
- **Impact**: Could delay mesh radio operations during initialization
- **Fix**: Removed blocking delays, made WiFi connection fully asynchronous
### 2. **Aggressive Processing Limits**
- **Issue**: Processing 2 packets per loop with 50ms time budget could still impact responsiveness
- **Impact**: Large packets (neighbors list) could consume significant CPU time
- **Fix**: Reduced to 1 packet per loop, reduced time budget to 30ms
### 3. **Memory Usage**
- **Issue**: Large stack allocations (2048 byte buffers) and unconditional raw data storage
- **Impact**: Increased stack pressure, unnecessary memory usage
- **Fix**: Reduced default buffer sizes, conditional raw data storage
### 4. **WiFi Reconnection Frequency**
- **Issue**: WiFi status checked every 10 seconds, reconnection attempts every 30 seconds
- **Impact**: Unnecessary CPU cycles and potential blocking
- **Fix**: Reduced check frequency to 15 seconds, reconnection attempts to 60 seconds
### 5. **Processing Overhead**
- **Issue**: Excessive debug logging and redundant checks in hot paths
- **Impact**: CPU cycles wasted on logging and checks
- **Fix**: Removed debug logging from hot paths, added early exit conditions
## Optimizations Applied
### 1. Non-Blocking Initialization
```cpp
// Before: Blocking WiFi wait
while (WiFi.status() != WL_CONNECTED && attempts < 20) {
delay(500); // BLOCKS ENTIRE SYSTEM
}
// After: Async connection
WiFi.begin(...);
// Auto-reconnect handles connection in background
```
### 2. Reduced Processing Limits
- **Packets per loop**: 2 → 1
- **Time budget**: 50ms → 30ms
- **Rationale**: Prioritize repeater responsiveness over MQTT throughput
### 3. Memory Optimizations
- **Default JSON buffer**: 1024 → 512 bytes (for small packets)
- **Raw data storage**: Only when `_packets_enabled` is true
- **Early queue checks**: Drop packets immediately if queue is full
### 4. Reduced WiFi Overhead
- **Status check interval**: 10s → 15s
- **Reconnection attempt interval**: 30s → 60s
- **Removed blocking delay**: WiFi reconnection is now fully async
### 5. Processing Optimizations
- **Early exit conditions**: Fast path for disabled/invalid states
- **Removed debug logging**: From hot paths (`onPacketReceived`, `processPacketQueue`)
- **Optimized topic building**: Build once, reuse for multiple brokers
## Remaining Considerations
### 1. NTP Sync Blocking
- **Status**: Acceptable - only called:
- During `begin()` after WiFi connects (async, non-critical path)
- Periodically in `loop()` every hour (acceptable blocking)
- **Note**: `NTPClient::forceUpdate()` has internal timeout, won't block indefinitely
### 2. JSON Buffer Stack Usage
- **Status**: Acceptable - 2048 bytes on stack is reasonable for ESP32 (8KB+ stack)
- **Note**: Stack allocation is faster than heap allocation and doesn't fragment memory
### 3. Packet Queue Size
- **Status**: Current limit (10 packets) is reasonable
- **Note**: Queue drops oldest packets when full, preventing memory growth
### 4. Multiple Broker Publishes
- **Status**: Acceptable - publishes are async (non-blocking)
- **Note**: PsychicMqttClient handles publishes asynchronously
## Performance Impact
### Before Optimizations
- WiFi initialization: **~10 seconds blocking**
- Packet processing: **Up to 2 packets, 50ms per loop**
- Memory: **Always storing raw data, large buffers**
### After Optimizations
- WiFi initialization: **Non-blocking (async)**
- Packet processing: **1 packet, 30ms max per loop**
- Memory: **Conditional raw data, smaller default buffers**
- WiFi overhead: **50% reduction in check frequency**
## Testing Recommendations
1. **Repeater Responsiveness**: Verify mesh radio operations aren't delayed during:
- WiFi connection/disconnection
- High packet rate scenarios
- MQTT broker reconnection
2. **Memory Usage**: Monitor stack usage during:
- Large packet processing (neighbors list)
- High queue scenarios
- Long-running operation
3. **MQTT Throughput**: Verify packets are still published correctly with:
- Reduced processing limits
- Smaller buffers
- Conditional raw data storage
## Code Changes Summary
### Files Modified
- `src/helpers/bridges/MQTTBridge.cpp`
- Removed blocking `delay()` calls
- Reduced processing limits (1 packet, 30ms)
- Optimized WiFi reconnection
- Added early exit conditions
- Removed debug logging from hot paths
- Conditional raw data storage
- Optimized topic building
- `src/helpers/MQTTMessageBuilder.cpp`
- Added comment about stack allocation efficiency
### No Regressions
- All existing functionality preserved
- MQTT publishing still works correctly
- Analyzer server support unchanged
- Status publishing unchanged
## Conclusion
The optimizations prioritize repeater responsiveness while maintaining MQTT functionality. The changes reduce blocking operations, minimize processing overhead, and optimize memory usage without introducing regressions.
-118
View File
@@ -1,118 +0,0 @@
# MQTT Connection Stability & CLI Responsiveness Review
## Summary
This document identifies issues in the MQTT codebase that could cause connection instability or unresponsive CLI behavior.
## Critical Issues Fixed
### 1. **Blocking Delays in Main Loop** ✅ FIXED
- **Location**: `connectToBrokers()` line 743, `loop()` line 475
- **Issue**: `delay(100)` calls block the entire system for 100ms during:
- Health check reconnections (every 4 hours)
- WiFi reconnection attempts
- **Impact**:
- Blocks CLI responsiveness
- Delays mesh radio operations
- Causes system-wide pauses
- **Fix**: Removed blocking delays - `disconnect()` and `connect()` are async operations
### 2. **Blocking WiFi Initialization** ✅ FIXED
- **Location**: `begin()` line 215-219
- **Issue**: Blocking `delay(500)` loop for up to 10 seconds (20 attempts) during WiFi initialization
- **Impact**:
- Blocks system startup
- Delays mesh radio initialization
- CLI unresponsive during startup
- **Fix**: Removed blocking wait - WiFi connection is now fully asynchronous
### 3. **Token Renewal Disconnection Strategy** ✅ FIXED (Previous Change)
- **Location**: `maintainAnalyzerConnections()` lines 1947-1958, 2052-2063
- **Issue**: Tokens were renewed 1 hour before expiration, causing unnecessary disconnections
- **Impact**: Connection drops every 23 hours
- **Fix**: Changed to renew only 60 seconds before expiration, only disconnect when token is actually expired
## Remaining Issues & Recommendations
### 4. **Static Variable Sharing in Health Check** ⚠️ MINOR
- **Location**: `connectToBrokers()` line 727
- **Issue**: `static unsigned long last_health_check` is shared across all brokers
- **Impact**: If multi-broker support is added, all brokers will trigger health checks simultaneously
- **Recommendation**: Use per-broker health check timestamps (store in broker struct)
- **Priority**: Low (current code only supports one broker at a time)
### 5. **NTP Sync Blocking** ⚠️ ACCEPTABLE
- **Location**: `syncTimeWithNTP()` line 2178
- **Issue**: `forceUpdate()` is a blocking call that can take several seconds
- **Impact**: Blocks main loop during NTP sync (every hour)
- **Status**: Acceptable - only happens once per hour, has internal timeout
- **Recommendation**: Consider making NTP sync fully async if responsiveness becomes an issue
### 6. **vTaskDelay in Packet Processing** ⚠️ ACCEPTABLE
- **Location**: `processPacketQueue()` line 833
- **Issue**: `vTaskDelay(1)` yields for ~10ms on ESP32 after each packet
- **Impact**: Adds latency to packet processing
- **Status**: Acceptable - prevents blocking, maintains responsiveness
- **Note**: This is intentional to yield to other tasks
### 7. **Connection State Race Conditions** ⚠️ POTENTIAL
- **Location**: Multiple locations checking `connected()` then using connection
- **Issue**: Connection state could change between check and use
- **Impact**: Potential for failed publishes or incorrect state tracking
- **Status**: Mitigated by async nature of PsychicMqttClient
- **Recommendation**: Monitor for any issues, consider adding connection state locks if problems occur
### 8. **Health Check Connection State Management** ✅ FIXED
- **Location**: `connectToBrokers()` line 754-756
- **Issue**: Health check reconnection didn't properly mark broker as disconnected
- **Impact**: Could cause state inconsistency
- **Fix**: Now properly marks broker as disconnected before async reconnection
## Performance Characteristics
### Before Fixes
- **WiFi initialization**: Up to 10 seconds blocking
- **Health check**: 100ms blocking every 4 hours
- **WiFi reconnect**: 100ms blocking during reconnect attempts
- **Token renewal**: Disconnection every 23 hours (1 hour before expiration)
### After Fixes
- **WiFi initialization**: Fully async, non-blocking
- **Health check**: Fully async, non-blocking
- **WiFi reconnect**: Fully async, non-blocking
- **Token renewal**: Disconnection only when token expires (within 60 seconds)
## Testing Recommendations
1. **Long-term stability test**: Run for 24+ hours and monitor:
- Connection uptime
- Queue length spikes
- Status publishing gaps
- CLI responsiveness
2. **Stress test**:
- Rapid WiFi disconnections/reconnections
- High packet rate
- Memory pressure scenarios
3. **CLI responsiveness test**:
- Issue CLI commands during:
- Token renewal
- Health check reconnection
- High packet processing
- WiFi reconnection
## Code Quality Notes
- All blocking operations have been removed from the main loop
- Connection operations are now fully asynchronous
- Token renewal strategy optimized to minimize disconnections
- Health check properly manages connection state
## Future Improvements
1. **Per-broker health check timestamps**: Store in broker struct instead of static variable
2. **Async NTP sync**: Consider making NTP sync fully async if responsiveness issues occur
3. **Connection state locking**: Add locks if race conditions become an issue
4. **Multi-broker support**: Implement proper multi-broker support with per-broker state management
-6725
View File
File diff suppressed because it is too large. Load diff