Skip to content

Implement WebSocket integration with Get Health endpoint#8

Merged
damnthonyy merged 4 commits into
developfrom
refactor/replace-client-rest-by-ws
May 21, 2026
Merged

Implement WebSocket integration with Get Health endpoint#8
damnthonyy merged 4 commits into
developfrom
refactor/replace-client-rest-by-ws

Conversation

@damnthonyy

Copy link
Copy Markdown
Member

Summary

Implement WebSocket integration with Get Health endpoint to replace static dashboard data with live robot telemetry (battery, ping, connection status).

Description

The dashboard was currently displaying hardcoded static data. This PR establishes a complete, cohesive WebSocket workflow that:

  • Connects to the robot backend on ws://localhost:8765
  • Sends a get_health request automatically on connection
  • Displays live robot state: battery level, ping latency, connection status
  • Provides a reusable pattern for future WebSocket routes (get_state, robot_state_updated, activity_event, etc.)

The implementation follows the protocol defined in docs/Frontend-comportement.md and integrates seamlessly with the existing architecture.

What's Changed

Affected areas:

  • WebSocket client abstraction layer
  • Robot gateway / service layer
  • React hooks for real-time state management
  • Dashboard header UI (live metrics)

Key files:

  • src/architecture/api/types.ts — Added WebSocket message types (HealthData, RobotState, WsIncomingMessage, WsOutgoingMessage)
  • src/architecture/gateways/robot.gateway.ts (new) — Singleton gateway wrapping WebSocketClient
  • src/hooks/use-robot-health.ts (new) — Custom React hook for WebSocket lifecycle management
  • src/components/dashboard/dashboard.tsx — Integrated useRobotHealth hook
  • src/components/dashboard/header.tsx — Replaced static text with live data binding

Details:

  • Discriminated union types ensure type-safe message routing
  • Singleton gateway pattern prevents duplicate connections
  • Automatic reconnection handled by existing WebSocketClient
  • Props-drilling pattern consistent with current architecture (no Context added)

How to Test

  • Start the dev server:
    npm run dev
    

Screen

  • Websocket opened
image
  • WebSocket closed
image

@damnthonyy damnthonyy requested a review from KelianHalleray May 20, 2026 14:10
@damnthonyy damnthonyy added refactor Code restructuring feature new functionality labels May 20, 2026
@qodo-code-review

Copy link
Copy Markdown

Review Summary by Qodo

Replace REST client with WebSocket for live robot telemetry
✨ Enhancement

Grey Divider

Walkthroughs

Description
• Replace REST client with WebSocket for real-time robot telemetry
• Add discriminated union types for type-safe WebSocket messaging
• Create RobotGateway singleton for centralized WebSocket management
• Implement useRobotHealth hook for live dashboard metrics
• Display dynamic battery, ping, and connection status in header
Diagram
flowchart LR
  A["Robot Backend<br/>ws://localhost:8765"] -->|WebSocket| B["WebSocketClient"]
  B -->|Singleton| C["RobotGateway"]
  C -->|useEffect| D["useRobotHealth Hook"]
  D -->|Props| E["Dashboard Header"]
  E -->|Display| F["Battery, Ping,<br/>Connection Status"]

Loading

File Changes

1. src/architecture/api/types.ts ✨ Enhancement +28/-0

Add WebSocket message types and robot state

src/architecture/api/types.ts


2. src/architecture/api/ws.client.ts ✨ Enhancement +4/-3

Update WebSocket URL configuration and protocol handling

src/architecture/api/ws.client.ts


3. src/architecture/api/client.test.ts Refactor +1/-1

Update import path to rest client

src/architecture/api/client.test.ts


View more (9)
4. src/architecture/api/ws.client.test.ts Refactor +6/-6

Update environment variable references to ROSBRIDGE_URL

src/architecture/api/ws.client.test.ts


5. src/architecture/gateways/robot.gateway.ts ✨ Enhancement +41/-0

Create singleton gateway for WebSocket communication

src/architecture/gateways/robot.gateway.ts


6. src/hooks/use-robot-health.ts ✨ Enhancement +45/-0

Implement custom hook for robot health WebSocket lifecycle

src/hooks/use-robot-health.ts


7. src/components/dashboard/dashboard.tsx ✨ Enhancement +8/-2

Integrate useRobotHealth hook and pass data to header

src/components/dashboard/dashboard.tsx


8. src/components/dashboard/header.tsx ✨ Enhancement +26/-6

Replace static metrics with live robot health data

src/components/dashboard/header.tsx


9. .env.example ⚙️ Configuration changes +1/-1

Update environment variable for WebSocket configuration

.env.example


10. .github/PULL_REQUEST_TEMPLATE.md 📝 Documentation +26/-10

Enhance PR template with better structure and guidance

.github/PULL_REQUEST_TEMPLATE.md


11. healhcheck.ts Additional files +0/-0

...

healhcheck.ts


12. src/architecture/api/rest.client.ts Additional files +0/-0

...

src/architecture/api/rest.client.ts


Grey Divider

Qodo Logo

@qodo-code-review

qodo-code-review Bot commented May 20, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (3) 📘 Rule violations (0)

Grey Divider


Remediation recommended

1. SetState after unmount 🐞 Bug ☼ Reliability
Description
useRobotHealth() disconnects the shared WebSocket on cleanup, but the WebSocketClient onclose
handler still invokes the hook’s callbacks which call setConnectionState, causing state updates
after the component unmounts. This produces React warnings and can leak resources on route
changes/unmounts.
Code

src/hooks/use-robot-health.ts[R27-37]

Evidence
The hook registers callbacks that update React state, and its cleanup closes the socket; the
WebSocket client always calls callbacks.onClose when a close event occurs, so a close triggered
during cleanup can still update state after unmount.

src/hooks/use-robot-health.ts[16-37]
src/architecture/api/ws.client.ts[89-107]
src/architecture/api/ws.client.ts[110-120]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`useRobotHealth()` registers `onClose`/`onError` callbacks that call `setConnectionState(...)`. On component cleanup it calls `robotGateway.disconnect()`, which closes the socket; the close event then triggers `onClose`, updating state after unmount.

### Issue Context
`WebSocketClient.disconnect()` closes the socket but does not remove callbacks before close events fire.

### Fix Focus Areas
- src/hooks/use-robot-health.ts[16-38]
- src/architecture/api/ws.client.ts[89-120]

### Suggested fix
Choose one (or combine):
1. In the hook, track an `isMounted`/`active` flag; in every callback, return early if inactive.
2. In `WebSocketClient.disconnect()`, clear callbacks (or add a `clearCallbacks()` API) before calling `socket.close()`.
3. Provide an unsubscribe/ref-count mechanism in `RobotGateway` so cleanup removes only that subscriber’s callbacks.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Ignored WS update messages 🐞 Bug ≡ Correctness
Description
useRobotHealth() only handles health_response and drops other defined WsIncomingMessage variants
like initial_state and robot_state_updated, so the hook will not update UI state from those
messages. This can leave battery/ping/connection telemetry stale if the backend uses those message
types for live updates.
Code

src/hooks/use-robot-health.ts[R22-26]

Evidence
The message union explicitly includes initial_state and robot_state_updated, but the hook’s
message handler only checks for health_response, so those other variants are ignored by design.

src/architecture/api/types.ts[67-72]
src/hooks/use-robot-health.ts[22-26]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`WsIncomingMessage` defines multiple message types, but `useRobotHealth()` only updates state for `health_response`, silently ignoring `initial_state` and `robot_state_updated`.

### Issue Context
This hook currently stores `HealthData` and displays `health.robot_state` in the header; if the backend emits robot state updates separately, the UI won’t react.

### Fix Focus Areas
- src/hooks/use-robot-health.ts[11-45]
- src/architecture/api/types.ts[67-72]

### Suggested fix
- Extend `onMessage` to handle `initial_state` and `robot_state_updated`.
 - Option A: Store `robotState` separately from `health` and update it on those messages.
 - Option B: Merge updates into `health.robot_state` when health is present.
- Consider handling `pong` (if it carries timing elsewhere) or triggering periodic `ping/get_health` if needed.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. Coverage include points to old file 🐞 Bug ◔ Observability
Description
Vitest coverage configuration still includes src/architecture/api/client.ts, but the REST client
is now in rest.client.ts (tests import it directly). This makes coverage reporting for the REST
client incorrect/missing.
Code

src/architecture/api/client.test.ts[2]

Evidence
The test suite now imports the REST client from ./rest.client, while the coverage config still
targets the old client.ts path, so the configured coverage target no longer matches the active
client module.

vitest.config.ts[10-19]
src/architecture/api/client.test.ts[1-3]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`vitest.config.ts` coverage `include` list still points at `src/architecture/api/client.ts`, while the code under test has moved to `src/architecture/api/rest.client.ts`.

### Issue Context
This doesn’t necessarily fail tests, but it will skew coverage metrics and may cause CI coverage checks to behave unexpectedly.

### Fix Focus Areas
- vitest.config.ts[10-19]

### Suggested fix
- Replace `src/architecture/api/client.ts` with `src/architecture/api/rest.client.ts` in `coverage.include`.
- Alternatively, switch to a glob like `src/architecture/api/*.client.ts` if you expect future client splits.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

@damnthonyy damnthonyy merged commit f07c1d5 into develop May 21, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature new functionality refactor Code restructuring

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants