Improve Bluetooth HRM Connection Resilience & State Management

[arii]

Original Issue Body

---

name: AI Refactor Request
about: Improve Bluetooth connection and retry handling with more generous reconnection logic
title: 'AI Task: Improve Bluetooth Connection Retry & State Management'
labels: 'auto-code'
assignees: ''

Refactoring Task

Improve the Bluetooth HRM connection and reconnection logic to be more resilient and generous in retry attempts. The current implementation has several state management issues and could be more aggressive in attempting to maintain connections.

Key Improvements Needed:

1. More Generous Reconnection Attempts

   • Current: 8 attempts with 2s base delay = 72s window
   • Increase to at least 12-15 attempts for a ~2-3 minute retry window
   • Consider exponential backoff for early attempts, then linear for later attempts

2. Proper GATT Cleanup & Restart

   • Ensure gatt.disconnect() is called before attempting reconnection
   • Add explicit cleanup of GATT server references before reconnect attempts
   • Consider calling device.forget() (if available) on persistent connection failures

3. Event Listener Management Issues

   • Lines 387-393, 463-482: Cleanup logic in useEffect and connectToGatt has potential race conditions
   • The activeDisconnectListenerRef pattern attempts to prevent duplicate listeners but may not handle all edge cases
   • Event listeners should be properly removed when:
     • Component unmounts
     • Before adding new listeners during reconnection
     • On manual disconnect
     • On device forget

4. State Synchronization Issues

   • Lines 85-94: Multiple refs tracking connection state (isManualDisconnect, isTimeoutDisconnect, isConnecting, etc.) can get out of sync
   • Lines 315-330: The reconnect callback checks statusRef.current in a timeout, creating potential race conditions
   • Consider consolidating into a single state machine or ensuring atomic state transitions

5. Abort Controller Handling

   • Lines 425-433: Creating multiple abort controllers without proper cleanup
   • Line 448: Duplicate AbortController creation (already created at line 437)
   • Ensure only one active abort controller exists at a time
   • Clean up abort controllers before creating new ones

6. Connection Flow Issues

   • Lines 415-424: The isConnecting guard may prevent legitimate reconnection attempts
   • Lines 456-459: Abort check after connection but server might already be connected
   • Lines 600-608: GATT disconnection errors don't trigger proper cleanup before reconnect

7. Timeout Handling

   • Lines 574-599: Connection timeout immediately exhausts reconnection attempts
   • Should attempt multiple timeouts before giving up
   • Consider increasing timeout from 20s to 30-40s for first attempt

Proposed Solutions

1. State Machine Refactor

   enum ConnectionState {
     IDLE,
     CONNECTING,
     CONNECTED,
     RECONNECTING,
     DISCONNECTING,
     ERROR
   }

   • Use single source of truth for connection state
   • Ensure valid state transitions only

2. Cleanup Function

   const cleanupGattConnection = async (device: BluetoothDevice) => {
     // Remove event listeners
     // Disconnect GATT if connected
     // Clear characteristic subscriptions
     // Reset abort controller
   }

3. Enhanced Reconnection Strategy

   • First 3 attempts: 2s, 4s, 8s (exponential for quick recovery)
   • Attempts 4-12: 5s intervals (linear for device boot time)
   • Total window: ~70s
   • Or increase to 15 attempts with 5s linear delay = 75s window

4. Event Listener Improvements

   • Use WeakMap to track listeners per device
   • Always remove listener before adding new one
   • Add listener cleanup to disconnect() function
   • Add listener cleanup before device forget

5. Better Error Recovery

   • Don't immediately give up on timeout
   • Retry GATT connection with fresh disconnect/reconnect cycle
   • Log all state transitions for debugging
   • Add telemetry for connection failure patterns

Target Files

- hooks/useBluetoothHRM.ts (lines 85-94, 216-236, 283-334, 336-368, 370-408, 415-618)
- constants/bluetooth-reconnection.ts (lines 10-17)
- constants/bluetooth-config.ts (add connection timeout constant)
- types/bluetooth.ts (consider adding ConnectionState enum)

Architecture Context

This refactor adheres to:

• Single Source of Truth Principle: Ensure Bluetooth state is authoritative and consistently managed
• Type Safety: No any types, use discriminated unions for connection states
• Code Conciseness: Reduce LOC by consolidating duplicate logic and simplifying state management

Current Issues Observed

1. Event Listener Leaks: activeDisconnectListenerRef pattern is complex and may not prevent all leaks across component remounts
2. Race Conditions: Multiple async operations (connectToGatt, reconnect, timeouts) can interfere with each other
3. State Inconsistency: Refs and state can diverge, especially during rapid connect/disconnect cycles
4. Insufficient Retry Window: 72s may be too short for devices that need to boot up or are temporarily out of range
5. Premature Failure: Connection timeout immediately exhausts all retry attempts instead of being one retry attempt
6. Abort Controller Duplication: Line 448 creates a second AbortController when one already exists from line 437

Expected Behavior After Refactor

1. Resilient Reconnection: Should maintain connection attempts for 2-3 minutes before giving up
2. Clean State Transitions: No race conditions or inconsistent state across refs and React state
3. Proper Cleanup: GATT connections and event listeners are always cleaned up properly
4. Better UX: Clear status messages indicating retry progress ("Reconnecting attempt 5 of 15...")
5. Predictable Behavior: State machine ensures only valid transitions occur
6. No Memory Leaks: Event listeners are properly removed on unmount and reconnection

Testing Considerations

• Test rapid connect/disconnect cycles
• Test component remount during active connection
• Test device going out of range and coming back
• Test device power cycle during active connection
• Test browser navigation away and back
• Verify no memory leaks with event listeners
• Verify abort controllers are properly cleaned up

Custom Branch Name (Optional)

branch-name: feature/improve-bluetooth-reconnection

---

Priority: High - Affects core functionality and user experience during workouts

Related ADR: ADR-0007: Simplified Bluetooth HRM Reconnection Strategy

Refactoring Task

Improve the Bluetooth HRM connection and reconnection logic to be more resilient and generous in retry attempts. The current implementation has several state management issues and could be more aggressive in attempting to maintain connections.

Key Improvements Needed:

1. More Generous Reconnection Attempts

   • Current: 8 attempts with 2s base delay = 72s window
   • Increase to at least 12-15 attempts for a ~2-3 minute retry window
   • Consider exponential backoff for early attempts, then linear for later attempts

2. Proper GATT Cleanup & Restart

   • Ensure gatt.disconnect() is called before attempting reconnection
   • Add explicit cleanup of GATT server references before reconnect attempts
   • Consider calling device.forget() (if available) on persistent connection failures

3. Event Listener Management Issues

   • Lines 387-393, 463-482: Cleanup logic in useEffect and connectToGatt has potential race conditions
   • The activeDisconnectListenerRef pattern attempts to prevent duplicate listeners but may not handle all edge cases
   • Event listeners should be properly removed when:
     • Component unmounts
     • Before adding new listeners during reconnection
     • On manual disconnect
     • On device forget

4. State Synchronization Issues

   • Lines 85-94: Multiple refs tracking connection state (isManualDisconnect, isTimeoutDisconnect, isConnecting, etc.) can get out of sync
   • Lines 315-330: The reconnect callback checks statusRef.current in a timeout, creating potential race conditions
   • Consider consolidating into a single state machine or ensuring atomic state transitions

5. Abort Controller Handling

   • Lines 425-433: Creating multiple abort controllers without proper cleanup
   • Line 448: Duplicate AbortController creation (already created at line 437)
   • Ensure only one active abort controller exists at a time
   • Clean up abort controllers before creating new ones

6. Connection Flow Issues

   • Lines 415-424: The isConnecting guard may prevent legitimate reconnection attempts
   • Lines 456-459: Abort check after connection but server might already be connected
   • Lines 600-608: GATT disconnection errors don't trigger proper cleanup before reconnect

7. Timeout Handling

   • Lines 574-599: Connection timeout immediately exhausts reconnection attempts
   • Should attempt multiple timeouts before giving up
   • Consider increasing timeout from 20s to 30-40s for first attempt

Proposed Solutions

1. State Machine Refactor

enum ConnectionState {
IDLE,
CONNECTING,
CONNECTED,
RECONNECTING,
DISCONNECTING,
ERROR
}
```
* Use single source of truth for connection state
* Ensure valid state transitions only

2. Cleanup Function

const cleanupGattConnection = async (device: BluetoothDevice) => {
// Remove event listeners
// Disconnect GATT if connected
// Clear characteristic subscriptions
// Reset abort controller
}
```

3. Enhanced Reconnection Strategy

   • First 3 attempts: 2s, 4s, 8s (exponential for quick recovery)
   • Attempts 4-12: 5s intervals (linear for device boot time)
   • Total window: ~70s
   • Or increase to 15 attempts with 5s linear delay = 75s window

4. Event Listener Improvements

   • Use WeakMap to track listeners per device
   • Always remove listener before adding new one
   • Add listener cleanup to disconnect() function
   • Add listener cleanup before device forget

5. Better Error Recovery

   • Don't immediately give up on timeout
   • Retry GATT connection with fresh disconnect/reconnect cycle
   • Log all state transitions for debugging
   • Add telemetry for connection failure patterns

Target Files

- hooks/useBluetoothHRM.ts (lines 85-94, 216-236, 283-334, 336-368, 370-408, 415-618)
- constants/bluetooth-reconnection.ts (lines 10-17)
- constants/bluetooth-config.ts (add connection timeout constant)
- types/bluetooth.ts (consider adding ConnectionState enum)

Architecture Context

This refactor adheres to:

• Single Source of Truth Principle: Ensure Bluetooth state is authoritative and consistently managed
• Type Safety: No any types, use discriminated unions for connection states
• Code Conciseness: Reduce LOC by consolidating duplicate logic and simplifying state management

Current Issues Observed

1. Event Listener Leaks: activeDisconnectListenerRef pattern is complex and may not prevent all leaks across component remounts
2. Race Conditions: Multiple async operations (connectToGatt, reconnect, timeouts) can interfere with each other
3. State Inconsistency: Refs and state can diverge, especially during rapid connect/disconnect cycles
4. Insufficient Retry Window: 72s may be too short for devices that need to boot up or are temporarily out of range
5. Premature Failure: Connection timeout immediately exhausts all retry attempts instead of being one retry attempt
6. Abort Controller Duplication: Line 448 creates a second AbortController when one already exists from line 437

Expected Behavior After Refactor

1. Resilient Reconnection: Should maintain connection attempts for 2-3 minutes before giving up
2. Clean State Transitions: No race conditions or inconsistent state across refs and React state
3. Proper Cleanup: GATT connections and event listeners are always cleaned up properly
4. Better UX: Clear status messages indicating retry progress ("Reconnecting attempt 5 of 15...")
5. Predictable Behavior: State machine ensures only valid transitions occur
6. No Memory Leaks: Event listeners are properly removed on unmount and reconnection

Testing Considerations

• Test rapid connect/disconnect cycles
• Test component remount during active connection
• Test device going out of range and coming back
• Test device power cycle during active connection
• Test browser navigation away and back
• Verify no memory leaks with event listeners
• Verify abort controllers are properly cleaned up

Custom Branch Name (Optional)

branch-name: feature/improve-bluetooth-reconnection

---

Priority: High - Affects core functionality and user experience during workouts

Related ADR: ADR-0007: Simplified Bluetooth HRM Reconnection Strategy

[google-labs-jules]

Jules is on it. When finished, you will see another comment and be able to review a PR.

[arii]

🤖 Gemini Smart Triage

Summary: Addresses critical Bluetooth HRM connection resilience and state management issues, including race conditions, event listener leaks, and an insufficient retry mechanism.
Priority: priority:high | Score: 10/10

Analysis:
This task requires a significant refactor of the Bluetooth HRM connection logic within useBluetoothHRM.ts. The primary focus will be on implementing a robust state machine to manage connection states atomically, enhancing the reconnection strategy with more generous exponential and linear backoff, ensuring proper cleanup of GATT connections and event listeners, and refining the handling of AbortController instances and connection timeouts. The refactor aims to eliminate race conditions, prevent state inconsistencies, and improve the overall user experience during connection attempts and disconnections.

Suggestion: Consider renaming this issue to: "Improve Bluetooth HRM Connection Resilience & State Management"

[google-labs-jules]

Ready for a review! A PR has been created.