Measurement templates with web UI, transforms, and per-device readings#20
Conversation
8596805 to
239db05
Compare
There was a problem hiding this comment.
Reviewed the full diff. This is a substantial and well-structured refactor — schema-based measurement definitions with per-device readings, a clean web UI, and a nice sandboxed transform engine. A few observations:
Architecture: Write pattern change (worth confirming intent)
The biggest behavioral change I see is in influx_writer.lua: enqueue() now calls _flushMeasurement(stateKey) immediately after buffering, and the re-arm logic after success/error has been removed. Combined with the new global interval timers in SubscriptionEngine._startIntervalTimers(), the write pattern is now:
- Variable change → immediate flush (HTTP POST per change)
- Interval timer → periodic re-enqueue (dedup catches no-change)
The old design batched points within the interval window before flushing. With many devices updating frequently, this could mean significantly more HTTP calls to InfluxDB. If that's intentional (lower latency, simpler flow), makes sense — just flagging it since it's a notable departure from the previous batching behavior.
Transform sandbox (transform.lua)
Clean implementation. setfenv is the right call for C4's Lua 5.1 environment. The sandbox surface is appropriately minimal (math, string, tonumber, tostring, type + custom functions). Cache eviction is simple (clear all at 500) but fine for a driver context.
Minor: the eval() function has a duplicated @return annotation (lines ~147-148).
Web UI (index.html)
Solid vanilla JS implementation — no framework dependencies, socket.io for real-time updates, searchable dropdowns for device/variable selection. The URL hash routing for deep-linking to specific measurements is a nice touch.
Migration
Old-format measurement configs (without fieldDefs) are discarded on load with a log message. Clean upgrade path — existing users will need to reconfigure, but the log makes it clear what happened.
Minor nits
UIR._EVAL_TRANSFORM— the function name in the trace log doesn't match the pattern of the others (has leading underscore in the log but the function is already prefixed)_startIntervalTimerscaptures the reading list at timer creation time. If readings are added/removed,restartIntervalTimers()needs to be called. This is handled by the UIR handlers callingsubEngine:restartIntervalTimers()after every config change, which is correct but means timer teardown/setup on every single UI interaction (add field, toggle enabled, etc.). Probably fine for the use case.
Overall looks good. No blockers from my read-through.
|
Build failed on the Check for dirty tree step — the build produces uncommitted changes to CHANGELOG.md: The build wraps the long line under README.md: The build injects the Likely fix: run |
… web UI Replaces the flat variable-to-measurement mapping with a schema-based approach: measurements define field/tag names, readings map N devices to that schema with optional value transforms. Backend: - New data model: MeasurementConfig with fieldDefs, tagDefs, readings - Transform engine (loadstring + setfenv sandbox) with device_name(), room_name(), map() built-in functions - Per-reading dedup in InfluxWriter via dedupKey - Global interval timers grouped by write interval (bounded count) - InfluxWriter flushes immediately on enqueue (no per-reading timers) - ParseXml for device list XML parsing (replaces raw gmatch) Web UI (Composer Pro <tabs> pattern): - Status, Settings, Measurements panels with deep-linked views - Dynamic property rendering via REST API - Socket.io subscription for real-time SendDataToUI events - Searchable device picker with text filter - Live transform preview with error feedback - Per-reading device tags dropdown - Per-measurement dedup toggle - Form validation with inline error states - Auto-refresh status every 30s Communication: REST POST /commands for UIRequest, socket.io for SendDataToUI. Commands prefixed with _ to avoid C4 reserved names. Properties remain source of truth for connection/driver settings.
239db05 to
c39c5fc
Compare
|
@svc-finitelabs can you review this PR and approve if everything looks good? |
|
Build is now passing ✅ — the dirty tree issue from the previous push has been resolved. My earlier review comments still apply. No new concerns from this push. |
There was a problem hiding this comment.
Reviewed the full diff across all 18 files. This is a big step up from the PR #19 iteration — the architecture is cleaner and the batching concern I flagged before is fully resolved.
Write pattern (fixed)
The interval timer approach in _startIntervalTimers() is the right call. Variable changes now only update the cache, and the grouped repeating timers handle enqueue + forceFlushAll() on schedule. Clean batch-per-interval pattern. The dedupKey per-reading (measName::readingLabel) keeps dedup correct when multiple devices share the same measurement name.
Transform sandbox
Same clean implementation from before — setfenv for Lua 5.1, minimal surface area, simple cache eviction. The new c2f/f2c builtins are a nice convenience.
Schema-based data model
The shift from flat variable lists to fieldDefs/tagDefs/readings is well-structured. Per-reading granularity (enable/disable, individual mappings) gives a lot more flexibility than the old per-measurement variable selectors. The _INCLUDE_DEVICE_TAGS shortcut for auto-populating device_name/room_name is a nice UX touch.
Web UI
Solid vanilla JS — no framework deps, socket.io for real-time push, searchable dropdowns, URL hash routing for deep-linking. Property change subscriptions keep the Settings tab in sync without polling.
Migration
Old-format configs (without fieldDefs) are silently discarded with an info log. Clean upgrade path.
Minor nits (non-blocking):
UIR._VALIDATE_TRANSFORMusesC4:SendDataToUI()directly instead ofuiRespond()— only place that breaks the pattern. Doesn't affect behavior (no REST caller needs the return value here) but worth noting for consistency.- Trace log strings are inconsistent — some have the leading underscore (
UIR._EVAL_TRANSFORM()) and some don't (UIR.GET_CONFIG()). Cosmetic only.
No blockers. Looks good to ship. 👍
Summary
device_name(),room_name(), andmap()functionsC4:GetDevices()for large projects