Skip to content

Fix device state reporting - handle both Integration and Legacy API formats - #2

Merged
ry-ops merged 1 commit into
mainfrom
fix/device-state-api-format-mismatch
Oct 30, 2025
Merged

Fix device state reporting - handle both Integration and Legacy API formats#2
ry-ops merged 1 commit into
mainfrom
fix/device-state-api-format-mismatch

Conversation

@ry-ops

@ry-ops ry-ops commented Oct 30, 2025

Copy link
Copy Markdown
Owner

🐛 Issue Description

All network devices were incorrectly reported as "unknown" state on MCP server startup, despite being online and functioning normally. This affected all device health monitoring functions.

Symptoms

{
  "by_state": { "unknown": 6 },
  "online_devices": 0,
  "total_devices": 6
}

Impact: Device health monitoring, status dashboards, and automation workflows received incorrect data about device states.


🔍 Root Cause Analysis

The Problem

Field type mismatch between UniFi's Integration API and Legacy API response formats:

API Type State Field Format Example
Integration API String "state": "ONLINE"
Legacy API Integer "state": 1

Code Issue

The device state mapping only handled integer values:

# Old code - only handled integers
state_name = {1: "online", 0: "offline", -1: "error"}.get(state, "unknown")

Result: When Integration API returned "ONLINE" (string), the .get() fell through to default "unknown".


🔬 Investigation Method

1. Verify Devices Are Actually Online

# Direct Integration API call
GET /sites/{site_id}/devices
# Response showed: state = "ONLINE" (string) ✅

2. Inspect Raw API Response

{
  "id": "...",
  "name": "Dream Machine Pro",
  "state": "ONLINE",  // ← String, not integer!
  "model": "UDM Pro"
}

3. Trace Code Execution

  • Found state mapping at main.py:598
  • Identified integer-only logic: {1: "online", 0: "offline"}.get(state, "unknown")
  • Located 2 additional functions with same issue

4. Replicate Issue

  • Cleared status cache
  • Called get_device_health() → returned "unknown": 6
  • Confirmed all devices showed "unknown" despite being online

✅ Fix Applied

Updated 3 functions to handle both API formats:

1. get_device_health_summary() (lines 596-622)

Before:

state = device.get("state", "unknown")
state_name = {1: "online", 0: "offline", -1: "error"}.get(state, "unknown")

After:

state = device.get("state", "unknown")

# Normalize state to lowercase string for consistent handling
if isinstance(state, str):
    state_name = state.lower()
elif isinstance(state, int):
    state_name = {1: "online", 0: "offline", -1: "error"}.get(state, "unknown")
else:
    state_name = "unknown"

Benefits:

  • Handles Integration API: "ONLINE""online"
  • Handles Legacy API: 1"online"
  • Type-safe with isinstance() checks
  • Also improved device type detection using model field as fallback

2. _collect_all_status() (lines 487-489)

Updated online device counting logic:

online_devices = [d for d in devices if
                 (isinstance(d.get("state"), int) and d.get("state") == 1) or
                 (isinstance(d.get("state"), str) and d.get("state").upper() == "ONLINE")]

3. get_quick_status() (lines 916-918)

Same fix applied to quick status function for consistency.


✅ Verification & Testing

Before Fix

{
  "total_devices": 6,
  "by_state": { "unknown": 6 },
  "by_type": { "unknown": 6 },
  "issues": [],
  "online_devices": 0
}

After Fix

{
  "total_devices": 6,
  "by_state": { "online": 6 },
  "by_type": {
    "UDM Pro": 1,
    "U7 Pro": 1,
    "U6+": 1,
    "Cable Internet": 1,
    "USP PDU Pro": 1,
    "USW Enterprise 24 PoE": 1
  },
  "issues": [],
  "online_devices": 6
}

Test Cases Verified

  • ✅ Integration API responses (string state)
  • ✅ Legacy API responses (integer state)
  • ✅ Device type detection with model field
  • ✅ Offline device detection (both string "OFFLINE" and int 0)
  • ✅ Status caching still works correctly
  • ✅ All 3 status functions return consistent data

📊 Impact Assessment

What This Fixes

  • ✅ Device health monitoring now shows accurate online/offline status
  • ✅ System status dashboard displays correct device states
  • ✅ Device type categorization works properly
  • ✅ Automated workflows can rely on accurate device status
  • ✅ MCP tools return correct device information to AI agents

Backwards Compatibility

  • ✅ Still supports Legacy API (integer states)
  • ✅ Fully supports Integration API (string states)
  • ✅ No breaking changes to function signatures
  • ✅ Existing code using these functions continues to work

Performance

  • ⚡ No performance impact (simple type checking)
  • ⚡ Caching behavior unchanged
  • ⚡ Same number of API calls

🧪 Testing Recommendations

Manual Testing

# Test device health
uv run python -c "from main import get_device_health; import json; print(json.dumps(get_device_health(), indent=2))"

# Test system status
uv run python -c "from main import get_system_status; import json; print(json.dumps(get_system_status(), indent=2))"

# Test quick status
uv run python -c "from main import get_quick_status; import json; print(json.dumps(get_quick_status(), indent=2))"

Expected Results

  • All devices should show as "online" (if actually online)
  • Device types should be properly categorized
  • online_devices count should match actual online devices

📝 Related

  • Issue Type: Bug Fix
  • Severity: High (affects core monitoring functionality)
  • API Compatibility: Integration API + Legacy API
  • UniFi Controller Versions: All versions supporting Integration API (7.0+)

🎯 Checklist

  • Root cause identified and documented
  • Fix applied to all affected functions
  • Code handles both API formats (Integration + Legacy)
  • Backwards compatibility maintained
  • Tested with both API response types
  • No breaking changes
  • Documentation updated (commit message)
  • Ready for review and merge

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Improved device status handling across different data formats for better consistency
    • Enhanced device health summary accuracy and reporting
    • Fixed device online/offline status detection reliability
    • Better device type identification and classification

…ormats

## Issue
All network devices were incorrectly reported as "unknown" state on server
startup, despite being online. This caused device health monitoring to show:
- by_state: { "unknown": 6 }
- online_devices: 0/6

## Root Cause
Field type mismatch between UniFi API formats:
- Integration API returns: state = "ONLINE" (string)
- Legacy API returns: state = 1 (integer)
- Code only handled integer values: {1: "online", 0: "offline", -1: "error"}
- Result: String values fell through to "unknown" default

## Investigation Method
1. Verified devices were actually online via direct Integration API call
2. Inspected raw API response showing state = "ONLINE" (string)
3. Traced code through get_device_health_summary() and found integer-only mapping
4. Identified 3 locations with same issue across status functions

## Fix Applied
Updated device state handling in 3 locations to support both API formats:

1. get_device_health_summary() (lines 596-622)
   - Normalize state to lowercase string for consistent handling
   - Check isinstance() for both string and integer
   - Handle both "ONLINE"/"OFFLINE" (Integration) and 1/0 (Legacy)
   - Improved device type detection using model field as fallback

2. _collect_all_status() (lines 487-489)
   - Updated online device counting logic
   - Handles both string "ONLINE" and integer 1

3. get_quick_status() (lines 916-918)
   - Fixed device online counting
   - Works with both API response formats

## Verification
Before: { "by_state": { "unknown": 6 }, "online_devices": 0 }
After:  { "by_state": { "online": 6 }, "online_devices": 6 }

All device types now correctly identified:
- UDM Pro, U7 Pro, U6+, USW Enterprise 24 PoE, USP PDU Pro, Cable Internet

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

Co-Authored-By: Claude <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 30, 2025

Copy link
Copy Markdown

Walkthrough

This change enhances device status collection to support both Integration API (string) and Legacy API (int) state representations. State handling, health summaries, device type resolution, offline detection, and online device counting logic have been updated to work with either format.

Changes

Cohort / File(s) Summary
Device State Handling
main.py
Enhanced state comparison logic across multiple functions to accept both integer (1, 0) and string ("ONLINE", "OFFLINE") representations. Updated online device filtering, device type resolution with fallback to "model", offline device detection, issue aggregation, and online device counting to handle dual state formats. State normalization applies lowercase transformation for string states while preserving int-to-state mappings.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Extra attention areas:
    • Verify state comparison logic consistency across all five modified functions (device collection, health summary, type resolution, offline check, and quick status flow)
    • Confirm string state comparisons are properly case-insensitive throughout
    • Validate fallback behavior when state is neither int nor recognized string format

Poem

🐰 A rabbit hops through digital lands,
Where states once spoke in just two commands—
But now they're flexible, unified, keen,
Speaking numbers, strings, and all in between!
Device harmony restored with care,

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title "Fix device state reporting - handle both Integration and Legacy API formats" directly and accurately summarizes the main change in this pull request. According to the PR objectives, the core issue is that devices were being reported as "unknown" due to a type mismatch between the Integration API (string states) and Legacy API (integer states), and this PR fixes that bug by normalizing both formats in state-handling logic. The title clearly captures both what is being fixed (device state reporting) and how it's being fixed (handling both API formats), making it specific and meaningful for someone scanning the repository history. It is concise, avoids unnecessary noise, and doesn't misrepresent the changeset.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/device-state-api-format-mismatch

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ceb79e8 and c3fc6be.

📒 Files selected for processing (1)
  • main.py (3 hunks)
🔇 Additional comments (4)
main.py (4)

486-489: LGTM! Dual-format state handling implemented correctly.

The online device filtering correctly handles both Integration API (string "ONLINE") and Legacy API (integer 1) formats. The isinstance guards prevent type errors, and the case-insensitive string comparison with .upper() ensures robustness.


613-613: Good improvement: device type fallback to model field.

The fallback from type to model field improves device type detection when the type field is missing, as mentioned in the PR objectives.


617-620: LGTM! Offline detection handles both API formats correctly.

The offline detection correctly checks the original state variable (not the normalized state_name) and handles both integer 0 and string "OFFLINE" formats with proper type guards and case-insensitive comparison.


918-921: LGTM! Consistent dual-format handling across functions.

The online device counting logic is consistent with the changes in _collect_all_status() (lines 486-489), correctly handling both API formats with the same type guards and comparison logic. This consistency ensures reliable device state reporting across the codebase.

Comment thread main.py
if isinstance(state, str):
state_name = state.lower()
elif isinstance(state, int):
state_name = {1: "online", 0: "offline", -1: "error"}.get(state, "unknown")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Fix syntax error: unquoted string literal.

The dictionary value error should be the string literal "error". This will cause a NameError at runtime when a device has state -1.

Apply this diff:

-                state_name = {1: "online", 0: "offline", -1: error}.get(state, "unknown")
+                state_name = {1: "online", 0: "offline", -1: "error"}.get(state, "unknown")
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
state_name = {1: "online", 0: "offline", -1: "error"}.get(state, "unknown")
state_name = {1: "online", 0: "offline", -1: "error"}.get(state, "unknown")
🤖 Prompt for AI Agents
In main.py around line 606, the dict mapping uses an unquoted identifier error
for the -1 key which will raise a NameError at runtime; change the mapping to
use the string "error" (i.e., ensure the value is quoted) so the dict becomes
{1: "online", 0: "offline", -1: "error"}.

@ry-ops
ry-ops merged commit 84cf4ab into main Oct 30, 2025
2 checks passed
@ry-ops
ry-ops deleted the fix/device-state-api-format-mismatch branch October 30, 2025 00:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant