Skip to content

Conversation

tonybentley
Copy link
Contributor

@tonybentley tonybentley commented Jun 19, 2025

Device Name Display Implementation Summary

Overview

Implemented a device name display feature for WebSocket connections in the SignalK Dashboard using a device registry cache approach. This addresses PR 2017 feedback about reducing UI complexity by integrating device names into the existing Dashboard rather than adding new menu items.

What Was Implemented

New Files Created

  1. src/deviceNameResolver.ts - Resolves client IDs to user-friendly names with 4-level priority
  2. src/deviceRegistryCache.ts - Event-driven in-memory cache for device registry data
  3. src/deviceRegistryCacheInit.ts - Initializes cache and sets up event listeners
  4. test/deviceNameResolver.js - Unit tests for name resolution logic (10 tests)

Modified Files

  1. src/interfaces/ws.js - Enhanced getActiveClients() to accept optional security context (backward compatible)
  2. src/deltastats.ts - Uses device registry cache to resolve display names with principal ID matching
  3. src/index.ts - Initializes device registry cache after security setup
  4. src/serverroutes.ts - Added event emissions for device updates/deletes
  5. packages/server-admin-ui/src/views/Dashboard/Dashboard.js - Shows display names in Connection & Plugin Activity

Architecture Implementation

Device Registry Cache

  • In-memory cache maintains copy of all registered devices
  • Event-driven updates via deviceUpdated and deviceRemoved events
  • Initializes at server startup from security.json
  • Thread-safe access to device information

Name Resolution Priority

  1. Device description from registry (e.g., "SensESP device: esp32-wireless")
  2. Principal name from authentication
  3. Parsed user agent (OpenCPN, SensESP, ESP32, Web Browser, etc.)
  4. Client ID as fallback

Event-Driven Updates

  • Device updates in Security → Devices emit events
  • Cache updates immediately upon receiving events
  • Dashboard reflects changes within 5 seconds (stats update interval)
  • Multiple browser sessions see updates simultaneously

Key Technical Details

ID Matching Solution

  • WebSocket providers use principal ID format: ws.{principal-id}
  • Device registry uses the same principal ID as clientId
  • Matching logic checks both WebSocket connection ID and principal identifier
  • Successfully resolves device descriptions when principal matches

Current Behavior

  • Device descriptions visible to all users (not just admins)
  • This is acceptable as device names are not sensitive information
  • Simplifies architecture by avoiding security context in background processes

Testing & Validation

  • ✅ Unit tests: 10/10 passing for device name resolution
  • ✅ TypeScript compilation successful
  • ✅ Visual validation confirmed device names display correctly
  • ✅ Event-driven updates validated with screen recording
  • ✅ Multi-browser real-time updates working

Uploading Screen Recording 2025-06-22 at 2.17.58 PM.mov…

Tony Bentley and others added 5 commits June 19, 2025 12:03
- Add getActiveClients() method to WebSocket interface
- Add /security/devices/active API endpoint with device name resolution
- Add Active Clients navigation menu and page component
- Display connected clients with friendly names instead of raw IDs

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
- Test /skServer/security/devices/active endpoint functionality
- Verify endpoint returns connected WebSocket clients with device info
- Test authentication requirements (admin access only)
- Validate response structure includes clientId, description, remoteAddress, etc.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
@KEGustafsson
Copy link
Contributor

KEGustafsson commented Jun 20, 2025

Related to ws id visibility on Dashboard, I made this kind of proposal to view devices.
#2013

@tkurki
Copy link
Member

tkurki commented Jun 21, 2025

No biggie, but for PRs with UI changes including screenshots makes it easier to get the overall impression.

@tkurki
Copy link
Member

tkurki commented Jun 21, 2025

Build failing with

 157 |                             {client.userAgent && client.description !== client.clientId && (
  158 |                               <br />
> 159 |                               <small className="text-muted" title={client.userAgent}>
      |                               ^
  160 |                                 {client.userAgent.length > 50 ? client.userAgent.substring(0, 50) + '...' : client.userAgent}
  161 |                               </small>
  162 |                             )}

@tkurki tkurki added the feature label Jun 21, 2025
Copy link
Member

@tkurki tkurki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the idea and alternative client naming scheme is sound, but I am just thinking that does it make sense to create one more view under Security or could this information be incorporated in the Devices list? More menu items and more views is more UI complexity for the user - like in what cases would you use the existing list view and when this?

There's also the overlap with the list in the Dashboard. Would it make sense to leverage that view? Additions there are potentially useful for all SK users, not just for those that have security active.

Another point is client-server comms: statistics on the dashboard and Data Browser update via WebScket without any user interaction. This view is showing real time status - what devices are online now, so as a user I would expect this view to update automatically, without reloading the view.

Tony Bentley and others added 2 commits June 21, 2025 19:04
- Add device name resolution utility from Active Clients feature
- Update deltastats to include display names for WebSocket providers
- Modify Dashboard to show friendly device names with ID fallback
- Remove Active Clients page, route, menu item and API endpoint
- Address PR SignalK#2017 review feedback to reduce UI complexity
- Resolves issue SignalK#1342 for WebSocket device identification

This change improves the Dashboard by showing meaningful device names
(e.g., 'OpenCPN', 'SensESP Device') instead of auto-generated IDs
(e.g., 'ws.c3f60a8f-c123-4b45-8de7') in the connections activity list.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
@tonybentley
Copy link
Contributor Author

tonybentley commented Jun 22, 2025

I'm going to take a stab at a different approach. The issue I am seeing is regarding the security context and how the dashboard, websocket interface, and delta stats are lacking security context. Instead of adding the new endpoint, I am going to inject security context. Here is a comprehensive plan. Let me know if you would like to keep the new endpoint. It might be useful for something, but not for what is needed to properly display device/client information based on security context.

@tonybentley tonybentley changed the title feat: add Active Clients feature to Security section feat: Device Name Display Implementation Jun 22, 2025
@tonybentley tonybentley marked this pull request as ready for review June 22, 2025 21:48
Copy link
Contributor Author

@tonybentley tonybentley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@tonybentley
Copy link
Contributor Author

@tkurki do you need anything to merge this PR? It's been open for over a month

@tkurki
Copy link
Member

tkurki commented Aug 3, 2025

Apologies for letting this and the one by @KEGustafsson sit here.

The reason is twofold:

  1. this one seems overly complex - why do we need this much more code? why do we need a cache, when the devices are listed in the settings file?
  2. the current Dashboard logic does not really reflect the cardinality of websockets - as the key is the device it does not really show if there are devices with active ws connections. this is something i’d like to fix properly, so that the list reflects the number of ws connections producing data. it is a bit problematic, as devices with ws connections are semantically a bit different from the server’s configured connections

So please wait a bit more while I have a go at redoing things a bit differently and circle back to this and #2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants