Skip to content

Fix per-device data collection, revert hourly to rate data#129

Merged
Yeraze merged 1 commit into
mainfrom
fix/125-device-collector-and-hourly-revert
Jun 25, 2026
Merged

Fix per-device data collection, revert hourly to rate data#129
Yeraze merged 1 commit into
mainfrom
fix/125-device-collector-and-hourly-revert

Conversation

@Yeraze

@Yeraze Yeraze commented Jun 25, 2026

Copy link
Copy Markdown
Owner

Summary

  • Collector fix: eero per-device data_usage/devices endpoint returns data under "values" key (not "devices"), and entries have flat upload/download daily totals — no hourly series breakdowns. Fixed key lookup and added fallback to flat fields.
  • Revert per-device hourly: since eero doesn't provide per-device hourly data, reverted the device bandwidth-history endpoint back to DeviceConnection rate snapshots (Mbps). This preserves hourly granularity on device pages.
  • Frontend revert: per-device charts use Mbps rate fields again.
  • Network-level hourly endpoint stays on HourlyBandwidth accumulated data (working correctly).
  • Bumps to v2.9.3.

Verified locally: collector now stores 69 devices per collection run. Per-device daily totals confirmed working (Sentry: 17.2 GB down, 20.0 GB up today).

Test plan

  • Deploy and verify per-device hourly charts show data again
  • Verify "Top Bandwidth Consumers" populates with daily per-device data
  • Verify network-level hourly chart still works from HourlyBandwidth
  • Check container logs for Updated data_usage for N devices

Authored by Merlin 🪄

…125)

- Fix collector: eero per-device endpoint uses "values" key (not "devices")
  and returns flat daily totals (no hourly series). Fixed key lookup and
  added fallback to flat upload/download fields when series is absent.
- Revert per-device bandwidth-history endpoint to DeviceConnection rate
  data since eero API doesn't provide per-device hourly breakdowns.
- Revert frontend per-device charts to Mbps rate-based rendering.
- Network-level hourly endpoint remains on HourlyBandwidth (unchanged).
- Version bump to 2.9.3.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@Yeraze
Yeraze merged commit 15e0920 into main Jun 25, 2026
@claude

claude Bot commented Jun 25, 2026

Copy link
Copy Markdown

Code Review — PR #129: Fix per-device data collection, revert hourly to rate data

This PR correctly identifies and fixes a real API mismatch (the eero per-device endpoint returns data under \"values\" with flat totals, not \"devices\" with hourly series) and reverts the device bandwidth-history endpoint to DeviceConnection rate snapshots since per-device hourly accumulated data isn't available. The logic flow is sound. Three issues worth addressing in a follow-up:


🔴 Unbounded query result set — src/api/health/routes.py ~line 869

Swapping from HourlyBandwidth to DeviceConnection increases result density from 24 rows/device/day (hourly aggregates) to 2,880 rows/device/day (one per 30-second poll interval). The query has no LIMIT or server-side aggregation:

connections = (
    db.query(DeviceConnection)
    .filter(DeviceConnection.device_id == device.id, DeviceConnection.timestamp >= cutoff_time)
    .order_by(DeviceConnection.timestamp.asc())
    .all()          # no LIMIT — returns all ~2,880 rows for a 24h window
)

A 168-hour (1-week) request returns ~20,000+ rows per device, serialising all of them into the JSON response. The frontend only bins them into 24 hourly buckets, so most of the resolution is thrown away client-side anyway. Consider server-side aggregation (GROUP BY hour) or at least a reasonable LIMIT.


🔴 TypeError crash when API returns {\"values\": null}src/collectors/data_usage_collector.py line 128

devices_list = result.get("values", result.get("devices", []))

When the key \"values\" exists but its value is null (JSON null → Python None), dict.get() returns None — the fallback default is never used because the key is present. The subsequent for device_entry in devices_list: then raises TypeError: 'NoneType' object is not iterable, crashing collection for that network silently (the exception propagates up and the batch stops). Fix:

devices_list = result.get("values", result.get("devices", [])) or []

🟡 Hardcoded secondsPerDataPoint = 30 in src/templates/devices.html lines 1572 & 1785

collection_interval_devices is already passed to dashboard.html via web.py and is user-configurable. The /devices route only passes {\"authenticated\": True, \"version\": ...} — it does not inject this variable into devices.html. As a result both chart functions hardcode 30, which silently produces wrong MB totals if someone sets a different collection interval. Fix: pass collection_interval_devices to the devices template and use {{ collection_interval_devices }} in both places.


🔵 Dead field count in hourlyDatasrc/templates/devices.html line 1568

count: 0 is initialised and incremented (hourlyData[hour].count++) but never read anywhere in the file. If it was intended for future averaging, it's not wired up yet; otherwise it can be removed.


Summary: the core data-format fix is correct and the local verification (69 devices, confirmed daily totals) looks solid. The unbounded query and the null-crash are the most actionable items for a follow-up patch.

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