Simplified Device Attributes Update Event (#206)

* core/device: Simplify device-update event triggering logic

* frontend: Remove device polling mechanism

* frontend: Fix linting issues

* frontend: Fix remarks from review

* core/api: Bump version to 1.3

Author: ccrisan

qtoggleserver/core/api/__init__.py

@@ -9,7 +9,7 @@
 from qtoggleserver.web import APIHandler
 
 
-API_VERSION = "1.1"
+API_VERSION = "1.3"
 
 ACCESS_LEVEL_ADMIN = 30
 ACCESS_LEVEL_NORMAL = 20

qtoggleserver/core/device/attrs.py

@@ -30,7 +30,7 @@
 
 
 EMPTY_PASSWORD_HASH = hashlib.sha256(b"").hexdigest()
-NETWORK_ATTRS_WATCH_INTERVAL = 5
+ATTRS_UPDATE_INTERVAL = 5
 ATTRDEF_CALLABLE_FIELDS = {"modifiable", "min", "max", "enabled"}
 
 
@@ -47,7 +47,7 @@
 _schema: GenericJSONDict | None = None
 _attrdefs_cache: AttributeDefinitions | None = None
 _to_json_attrdefs_cache: AttributeDefinitions | None = None
-_attrs_watch_task: asyncio.Task | None = None
+_attrs_update_task: asyncio.Task | None = None
 _attrs_cache: Attributes | None = None
 
 
@@ -906,62 +906,32 @@ async def to_json() -> GenericJSONDict:
     return result
 
 
-def _check_net_data_changed(data: dict) -> bool:
-    changed = False
-
-    if system.net.has_wifi_support():
-        wifi_config = system.net.get_wifi_config()
-        old_wifi_config = data.get("wifi_config")
-        if old_wifi_config != wifi_config:
-            data["wifi_config"] = wifi_config
-            changed = True
-
-    if system.net.has_ip_support():
-        ip_config = system.net.get_ip_config()
-        old_ip_config = data.get("ip_config")
-        if old_ip_config != ip_config:
-            data["ip_config"] = ip_config
-            changed = True
-
-    return changed
-
-
-async def _attrs_watch_loop() -> None:
-    # TODO: also watch dynamic attributes
-
-    last_net_data = {}
-
+async def _attrs_update_loop() -> None:
     try:
         while True:
-            changed = False
             try:
-                if _check_net_data_changed(last_net_data):
-                    logger.debug("network attributes data changed")
-                    changed = True
-            except Exception as e:
-                logger.error("network attributes data check failed: %s", e, exc_info=True)
-
-            if changed:
                 invalidate_attrs()
                 await device_events.trigger_update()
-
-            await asyncio.sleep(NETWORK_ATTRS_WATCH_INTERVAL)
+            except Exception:
+                logger.exception("Error updating attributes")
+            finally:
+                await asyncio.sleep(ATTRS_UPDATE_INTERVAL)
     except asyncio.CancelledError:
-        logger.debug("attributes watch task cancelled")
+        logger.debug("attributes update task cancelled")
 
 
 async def init() -> None:
-    global _attrs_watch_task
+    global _attrs_update_task
 
-    logger.debug("starting attributes watch task")
-    _attrs_watch_task = asyncio.create_task(_attrs_watch_loop())
+    logger.debug("starting attributes update task")
+    _attrs_update_task = asyncio.create_task(_attrs_update_loop())
 
 
 async def cleanup() -> None:
-    logger.debug("stopping attributes watch task")
-    if _attrs_watch_task:
-        _attrs_watch_task.cancel()
+    logger.debug("stopping attributes update task")
+    if _attrs_update_task:
+        _attrs_update_task.cancel()
         try:
-            await _attrs_watch_task
+            await _attrs_update_task
         except asyncio.CancelledError:
             pass

qtoggleserver/frontend/js/cache.js

@@ -10,14 +10,11 @@ import {gettext}         from '$qui/base/i18n.js'
 import Config            from '$qui/config.js'
 import * as Toast        from '$qui/messages/toast.js'
 import Debouncer         from '$qui/utils/debouncer.js'
-import {asap}            from '$qui/utils/misc.js'
 import * as ObjectUtils  from '$qui/utils/object.js'
 import * as PromiseUtils from '$qui/utils/promise.js'
-import * as Window       from '$qui/window.js'
 
 import * as AuthAPI               from '$app/api/auth.js'
 import * as APIConstants          from '$app/api/constants.js'
-import * as BaseAPI               from '$app/api/base.js'
 import * as DevicesAPI            from '$app/api/devices.js'
 import * as PortsAPI              from '$app/api/ports.js'
 import * as PrefsAPI              from '$app/api/prefs.js'
@@ -27,20 +24,6 @@ import * as NotificationsAPI      from '$app/api/notifications.js'
 import {getGlobalProgressMessage} from '$app/common/common.js'
 
 
-const DEVICE_POLL_INTERVAL = 5 /* Seconds */
-
-/* When actively polling a device, only update some selected attributes */
-const DEVICE_POLLED_ATTRIBUTES = [
-    'date',
-    'uptime',
-    'wifi_signal_strength',
-    'temperature',
-    'cpu_usage',
-    'mem_usage',
-    'storage_usage',
-    'battery_level'
-]
-
 const STORAGE_KEY_PREFIX = 'cache'
 const STORAGE_SET_DEBOUNCE_DELAY = 5000
 const SAVE_PREFS_DEBOUNCE_DELAY = 1000
@@ -70,9 +53,6 @@ const savePrefsDebouncer = new Debouncer(() => {
 /* Indicates that cache needs a reload asap */
 let reloadNeeded = false
 
-/* The name of a device to be continuously polled */
-let polledDeviceName = null
-
 /* Debouncers for cache setters */
 const slaveDevicesSetLocalStorageCacheDebouncer = new Debouncer((slaveDevices) => {
     logger.debug('saving slave devices to cache')
@@ -546,18 +526,6 @@ export function updateFromEvent(event) {
             break
         }
 
-        case 'slave-device-polling-update': {
-            if (event.params.name in slaveDevices) {
-                Object.assign(slaveDevices[event.params.name].attrs, event.params.attrs)
-                slaveDevicesSetLocalStorageCacheDebouncer.call(slaveDevices)
-            }
-            else {
-                logger.warn(`received slave-device-polling-update event for unknown device "${event.params.name}"`)
-            }
-
-            break
-        }
-
         case 'slave-device-add': {
             if (event.params.name in slaveDevices) {
                 logger.debug(`received slave-device-add event for already existing device "${event.params.name}"`)
@@ -579,10 +547,6 @@ export function updateFromEvent(event) {
                 logger.warn(`received slave-device-remove event for unknown device "${event.params.name}"`)
             }
 
-            if (polledDeviceName === event.params.name) {
-                polledDeviceName = null
-            }
-
             break
         }
 
@@ -649,14 +613,6 @@ export function updateFromEvent(event) {
 
             break
         }
-
-        case 'device-polling-update': {
-            Object.assign(mainDevice, event.params)
-            mainDeviceSetLocalStorageCacheDebouncer.call(mainDevice)
-
-            break
-        }
-
     }
 }
 
@@ -900,96 +856,6 @@ export function reload(now = false) {
     }
 }
 
-/**
- * Return the name of the polled device.
- * @alias qtoggle.cache.getPolledDeviceName
- * @returns {?String}
- */
-export function getPolledDeviceName() {
-    return polledDeviceName
-}
-
-/**
- * Set the name of the polled device. Passing `null` disables polling.
- * @alias qtoggle.cache.setPolledDeviceName
- * @param {?String} [deviceName]
- */
-export function setPolledDeviceName(deviceName) {
-    if (deviceName == null) {
-        logger.debug('disabling device polling')
-    }
-    else {
-        logger.debug(`setting polled device name to "${deviceName}"`)
-    }
-
-    polledDeviceName = deviceName
-}
-
-function pollDevice() {
-    /* Choose between polling main device or a slave device */
-    let device = null
-    if (polledDeviceName) {
-        logger.debug(`polling device "${polledDeviceName}"`)
-        device = slaveDevices[polledDeviceName]
-        if (!device) {
-            logger.debug('skipping polling for unknown device')
-            return
-        }
-
-        if (!device.enabled) {
-            logger.debug('skipping polling for disabled device')
-            return
-        }
-
-        BaseAPI.setSlaveName(polledDeviceName)
-    }
-    else {
-        logger.debug('polling main device')
-    }
-
-    DevicesAPI.getDevice().then(function (attrs) {
-
-        asap(function () {
-            if (device && polledDeviceName === device.name) {
-                if (!ObjectUtils.deepEquals(device.attrs, attrs)) {
-                    let partialDevice = {name: device.name, attrs: {}}
-                    DEVICE_POLLED_ATTRIBUTES.forEach(function (name) {
-                        if (name in attrs) {
-                            partialDevice.attrs[name] = attrs[name]
-                        }
-                    })
-                    NotificationsAPI.fakeServerEvent('slave-device-polling-update', partialDevice)
-                }
-            }
-            else if (polledDeviceName === '') {
-                if (!ObjectUtils.deepEquals(mainDevice, attrs)) {
-                    let partialAttrs = {}
-                    DEVICE_POLLED_ATTRIBUTES.forEach(function (name) {
-                        if (name in attrs) {
-                            partialAttrs[name] = attrs[name]
-                        }
-                    })
-                    NotificationsAPI.fakeServerEvent('device-polling-update', partialAttrs)
-                }
-            }
-        })
-
-    }).catch(function (e) {
-
-        if (polledDeviceName == null) {
-            logger.debug('ignoring polling error after polling disabled')
-            return
-        }
-        if ((e instanceof BaseAPI.APIError) && (e.code === 'no such device')) {
-            logger.debug('ignoring error while polling removed device')
-            return
-        }
-
-        logger.errorStack('polling failed', e)
-
-    })
-}
-
 /**
  * Initialize the cache subsystem.
  * @alias qtoggle.cache.init
@@ -1006,26 +872,4 @@ export function init() {
         }
 
     })
-
-    /* Start a polling timer */
-    setInterval(function () {
-
-        /* Don't poll unless cache is ready */
-        if (!whenCacheReady.isFulfilled()) {
-            return
-        }
-
-        /* Don't poll unless window currently active */
-        if (!Window.isActive()) {
-            return
-        }
-
-        /* Don't poll if polling device disabled */
-        if (polledDeviceName == null) {
-            return
-        }
-
-        pollDevice()
-
-    }, DEVICE_POLL_INTERVAL * 1000)
 }

qtoggleserver/frontend/js/common/reboot-device-mixin.js

@@ -67,12 +67,6 @@ const RebootDeviceMixin = Mixin((superclass = Object) => {
             this.setProgress()
             this._rebooting = true
 
-            /* Disable polling while rebooting */
-            let polledDeviceName = Cache.getPolledDeviceName()
-            if (polledDeviceName === deviceName) {
-                Cache.setPolledDeviceName(null)
-            }
-
             if (!Cache.isMainDevice(deviceName)) {
                 BaseAPI.setSlaveName(deviceName)
             }
@@ -107,11 +101,6 @@ const RebootDeviceMixin = Mixin((superclass = Object) => {
                 this.clearProgress()
                 this._rebooting = false
 
-                /* Restore polling */
-                if (polledDeviceName === deviceName) {
-                    Cache.setPolledDeviceName(deviceName)
-                }
-
             }.bind(this))
         }
 

qtoggleserver/frontend/js/devices/device-form.js

@@ -128,18 +128,6 @@ class DeviceForm extends mix(PageForm).with(
         this.updateUI(/* fieldChangeWarnings = */ false)
     }
 
-    onBecomeCurrent() {
-        if (this._deviceRemoved) {
-            return
-        }
-
-        Cache.setPolledDeviceName(this.getDeviceName())
-    }
-
-    onLeaveCurrent() {
-        Cache.setPolledDeviceName(null)
-    }
-
     /**
      * Update the entire form (fields & values) from the corresponding device.
      */
@@ -402,9 +390,6 @@ class DeviceForm extends mix(PageForm).with(
                     /* Device renamed, remember new name for reopening */
                     logger.debug(`device "${deviceName}" renamed to "${value}"`)
                     Devices.setRenamedDeviceName(value)
-
-                    /* Disable polling since it would soon poll an inexistent device name */
-                    Cache.setPolledDeviceName(null)
                 }
 
                 if (this._fullAttrdefs[name].reconnect) {

qtoggleserver/frontend/js/devices/devices-section.js

@@ -85,14 +85,6 @@ class DevicesSection extends Section {
                 break
             }
 
-            case 'slave-device-polling-update': {
-                if (deviceForm && (deviceForm.getDeviceName() === event.params.name) && !deviceForm.isRebooting()) {
-                    deviceForm.updateUI(/* fieldChangeWarnings = */ false)
-                }
-
-                break
-            }
-
             case 'slave-device-add': {
                 this.devicesTable.updateUIASAP()