Merge pull request #136 from agittins/agittins-dev

fix: Fix updates for local USB scan entries

- There was a bug where the device.scanner data wasn't being updated if the scanner was no longer providing (caching) an advertisement for that device. This mainly showed up on local usb bluetooth dongles, because I think BlueZ would expunge the advert history more quickly than we timeout a distance reading (30 sec by default). The result would be that out-of-range devices would snap to the last distance recorded by the usb adaptor, since we never cleaned it up previously. This might also result in other fixes but I haven't seen cases where proxies would drop advert caches.

- Removed have_new_stamp and refactored it to be indicated by new_stamp being None. Preserved into self.new_stamp and split the calculate_data() step out of t
he update_advertisement() step.

- linting: Changed FastFalling/Rising to snake_case.

custom_components/bermuda/__init__.py

@@ -153,8 +153,10 @@ def __init__(
         # which is a bit silly, I suspect.
         self.name: str = scandata.scanner.name
         self.area_id: str = area_id
+        self.parent_device = device_address
 
         self.stamp: float = 0
+        self.new_stamp = None  # Set when a new advert is loaded from update
         self.hist_stamp = []
         self.rssi: float = None
         self.hist_rssi = []
@@ -170,9 +172,9 @@ def __init__(
         self.rssi_distance_raw: float = None
 
         # Just pass the rest on to update...
-        self.update(device_address, scandata, area_id, options)
+        self.update_advertisement(device_address, scandata, area_id, options)
 
-    def update(
+    def update_advertisement(
         self,
         device_address: str,
         scandata: BluetoothScannerDevice,
@@ -183,14 +185,14 @@ def update(
         every time we do a polled update.
 
         This method needs to update all the history and tracking data for this
-        device+scanner combination.
+        device+scanner combination. This method only gets called when a given scanner
+        claims to have data.
         """
-        # We over-write pretty much everything, except our locally-preserved stats.
-        #
-        # In case the scanner has changed it's details since startup though:
+
+        # In case the scanner has changed it's details since startup:
         self.name: str = scandata.scanner.name
         self.area_id: str = area_id
-        have_new_stamp = False
+        new_stamp = None
 
         # Only remote scanners log timestamps here (local usb adaptors do not),
         if hasattr(scandata.scanner, "_discovered_device_timestamps"):
@@ -204,11 +206,9 @@ def update(
             uppermac = device_address.upper()
             if uppermac in stamps:
                 if stamps[uppermac] > self.stamp:
-                    have_new_stamp = True
                     new_stamp = stamps[uppermac]
                 else:
                     # We have no updated advert in this run.
-                    have_new_stamp = False
                     new_stamp = None
                     self.stale_update_count += 1
             else:
@@ -219,7 +219,6 @@ def update(
                     scandata.scanner.source,
                     device_address,
                 )
-                have_new_stamp = False
                 new_stamp = None
         else:
             # Not a bluetooth_proxy device / remote scanner, but probably a USB Bluetooth adaptor.
@@ -236,20 +235,18 @@ def update(
                 # proxies with stale adverts. Hopefully.
                 # new_stamp = MONOTONIC_TIME() - (ADVERT_FRESHTIME * 4)
                 new_stamp = MONOTONIC_TIME()
-                have_new_stamp = True
             else:
-                have_new_stamp = False
                 new_stamp = None
 
-        if len(self.hist_stamp) == 0 or have_new_stamp:
+        if len(self.hist_stamp) == 0 or new_stamp is not None:
             # this is the first entry or a new one...
 
             self.rssi: float = scandata.advertisement.rssi
             self.hist_rssi.insert(0, self.rssi)
             self.rssi_distance_raw: float = rssi_to_metres(
                 self.rssi,
-                options.get(CONF_REF_POWER, DEFAULT_REF_POWER),
-                options.get(CONF_ATTENUATION, DEFAULT_ATTENUATION),
+                options.get(CONF_REF_POWER),
+                options.get(CONF_ATTENUATION),
             )
             self.hist_distance.insert(0, self.rssi_distance_raw)
 
@@ -288,26 +285,54 @@ def update(
         self.adverts: dict[str, bytes] = scandata.advertisement.service_data.items()
         self.options = options
 
-        # ###### Filter and update distance estimates.
-        #
-        # Note: Noise in RSSI readings is VERY asymmetric. Ultimately,
-        # a closer distance is *always* more accurate than a previous
-        # more distant measurement. Any measurement might be true,
-        # or it is likely longer than the truth - and (almost) never
-        # shorter.
-        #
-        # For a new, long measurement to be true, we'd want to see some
-        # indication of rising measurements preceding it, or at least a
-        # long time since our last measurement.
-        #
-        # It's tempting to treat no recent measurement as implying an increase
-        # in distance, but doing so would wreak havoc when we later try to
-        # implement trilateration, so better to simply cut a sensor off as
-        # "away" from a scanner when it hears no new adverts. DISTANCE_TIMEOUT
-        # is how we decide how long to wait, and should accommodate for dropped
-        # packets and for temporary occlusion (dogs' bodies etc)
-
-        if have_new_stamp and self.rssi_distance is None:
+        self.new_stamp = new_stamp
+
+    def calculate_data(self):
+        """Filter and update distance estimates.
+
+        All smoothing and noise-management of the distance between a scanner
+        and a device should be done in this method, as it is
+        guaranteed to be called on every update cycle, for every
+        scanner that has ever reported an advert for this device
+        (even if it is not reporting one currently).
+
+        If new_stamp is None it implies that the scanner has not reported
+        an updated advertisement since our last update cycle,
+        so we may need to check if this device should be timed
+        out or otherwise dealt with.
+
+        If new_stamp is not None it means we just had an updated
+        rssi_distance_raw value which should be processed.
+
+        This is called by self.update, but should also be called for
+        any remaining scanners that have not sent in an update in this
+        cycle. This is mainly beacuse usb/bluez adaptors seem to flush
+        their advertisement lists quicker than we time out, so we need
+        to make sure we still update the scanner entry even if the scanner
+        no longer carries advert history for this device.
+
+        Note: Noise in RSSI readings is VERY asymmetric. Ultimately,
+        a closer distance is *always* more accurate than a previous
+        more distant measurement. Any measurement might be true,
+        or it is likely longer than the truth - and (almost) never
+        shorter.
+
+        For a new, long measurement to be true, we'd want to see some
+        indication of rising measurements preceding it, or at least a
+        long time since our last measurement.
+
+        It's tempting to treat no recent measurement as implying an increase
+        in distance, but doing so would wreak havoc when we later try to
+        implement trilateration, so better to simply cut a sensor off as
+        "away" from a scanner when it hears no new adverts. DISTANCE_TIMEOUT
+        is how we decide how long to wait, and should accommodate for dropped
+        packets and for temporary occlusion (dogs' bodies etc)
+        """
+
+        new_stamp = self.new_stamp  # should have been set by update()
+        self.new_stamp = None  # Clear so we know if an update is missed next cycle
+
+        if new_stamp is not None and self.rssi_distance is None:
             # DEVICE HAS ARRIVED!
             # We have just newly come into range (or we're starting up)
             # accept the new reading as-is.
@@ -316,7 +341,7 @@ def update(
             self.hist_distance_by_interval.insert(0, self.rssi_distance_raw)
             del self.hist_distance_by_interval[1:]
 
-        elif (not have_new_stamp) and self.stamp < MONOTONIC_TIME() - DISTANCE_TIMEOUT:
+        elif (new_stamp is None) and self.stamp < MONOTONIC_TIME() - DISTANCE_TIMEOUT:
             # DEVICE IS AWAY!
             # Last distance reading is stale, mark device distance as unknown.
             self.rssi_distance = None
@@ -345,7 +370,7 @@ def update(
                         continue
 
                     delta_t = velo_newstamp - self.hist_stamp[i]
-                    delta_d = velo_newdistance - self.hist_distance[i]
+                    delta_d = velo_newdistance - old_distance
                     velocity = delta_d / delta_t
 
                     # Approach velocities are only interesting vs the previous
@@ -371,10 +396,10 @@ def update(
             self.hist_velocity.insert(0, velocity)
 
             if velocity > self.options.get(CONF_MAX_VELOCITY):
-                if device_address.upper() in self.options[CONF_DEVICES]:
+                if self.parent_device.upper() in self.options[CONF_DEVICES]:
                     _LOGGER.debug(
                         "This sparrow %s flies too fast (%2fm/s), ignoring",
-                        device_address,
+                        self.parent_device,
                         velocity,
                     )
                 # Discard the bogus reading by duplicating the last.
@@ -493,18 +518,42 @@ def __init__(self, address, options):
         )
         self.scanners: dict[str, BermudaDeviceScanner] = {}
 
-    def add_scanner(
+    def calculate_data(self):
+        """Call after doing update_scanner() calls so that distances
+        etc can be freshly smoothed and filtered.
+
+        """
+        for scanner in self.scanners.values():
+            scanner.calculate_data()
+
+        # Update whether the device has been seen recently, for device_tracker:
+        if (
+            self.last_seen is not None
+            and MONOTONIC_TIME()
+            - self.options.get(CONF_DEVTRACK_TIMEOUT, DEFAULT_DEVTRACK_TIMEOUT)
+            < self.last_seen
+        ):
+            self.zone = STATE_HOME
+        else:
+            self.zone = STATE_NOT_HOME
+
+        if self.address.upper() in self.options.get(CONF_DEVICES, []):
+            # We are a device we track. Flag for set-up:
+            self.create_sensor = True
+
+    def update_scanner(
         self, scanner_device: BermudaDevice, discoveryinfo: BluetoothScannerDevice
     ):
-        """Add/Replace a scanner entry on this device, indicating a received advertisement
+        """Add/Update a scanner entry on this device, indicating a received advertisement
 
         This gets called every time a scanner is deemed to have received an advert for
-        this device.
+        this device. It only loads data into the structure, all calculations are done
+        with calculate_data()
 
         """
         if format_mac(scanner_device.address) in self.scanners:
             # Device already exists, update it
-            self.scanners[format_mac(scanner_device.address)].update(
+            self.scanners[format_mac(scanner_device.address)].update_advertisement(
                 self.address,
                 discoveryinfo,  # the entire BluetoothScannerDevice struct
                 scanner_device.area_id,
@@ -599,9 +648,13 @@ def __init__(
 
         # TODO: This is only here because we haven't set up migration of config
         # entries yet, so some users might not have this defined after an update.
+        self.options[CONF_ATTENUATION] = DEFAULT_ATTENUATION
+        self.options[CONF_DEVTRACK_TIMEOUT] = DEFAULT_DEVTRACK_TIMEOUT
         self.options[CONF_MAX_RADIUS] = DEFAULT_MAX_RADIUS
-        self.options[CONF_SMOOTHING_SAMPLES] = DEFAULT_SMOOTHING_SAMPLES
         self.options[CONF_MAX_VELOCITY] = DEFAULT_MAX_VELOCITY
+        self.options[CONF_REF_POWER] = DEFAULT_REF_POWER
+        self.options[CONF_SMOOTHING_SAMPLES] = DEFAULT_SMOOTHING_SAMPLES
+        self.options[CONF_UPDATE_INTERVAL] = DEFAULT_UPDATE_INTERVAL
 
         if hasattr(entry, "options"):
             # Firstly, on some calls (specifically during reload after settings changes)
@@ -839,22 +892,16 @@ async def _async_update_data(self):
                     )
                     continue
 
-                # Replace the scanner entry on the current device
-                device.add_scanner(scanner_device, discovered)
+                # Update the scanner entry on the current device
+                device.update_scanner(scanner_device, discovered)
 
-            # Update whether the device has been seen recently, for device_tracker:
-            if (
-                MONOTONIC_TIME()
-                - self.options.get(CONF_DEVTRACK_TIMEOUT, DEFAULT_DEVTRACK_TIMEOUT)
-                < device.last_seen
-            ):
-                device.zone = STATE_HOME
-            else:
-                device.zone = STATE_NOT_HOME
+            # END of per-advertisement-by-device loop
 
-            if device.address.upper() in self.options.get(CONF_DEVICES, []):
-                # This is a device we track. Flag it for set-up:
-                device.create_sensor = True
+        # Scanner entries have been loaded up with latest data, now we can process data for all devices
+        # over all scanners.
+        for device in self.devices.values():
+            # Recalculate smoothed distances, last_seen etc
+            device.calculate_data()
 
         self._refresh_areas_by_min_distance()
 

custom_components/bermuda/entity.py

@@ -48,7 +48,7 @@ def __init__(
         self.bermuda_last_state: Any = 0
         self.bermuda_last_stamp: int = 0
 
-    def _cached_ratelimit(self, statevalue: Any, FastFalling=True, FastRising=False):
+    def _cached_ratelimit(self, statevalue: Any, fast_falling=True, fast_rising=False):
         """Uses the CONF_UPDATE_INTERVAL and other logic to return either the given statevalue
         or an older, cached value. Helps to reduce excess sensor churn without compromising latency.
 
@@ -62,8 +62,10 @@ def _cached_ratelimit(self, statevalue: Any, FastFalling=True, FastRising=False)
             )  # Cache is stale
             or (self.bermuda_last_state is None)  # Nothing compares to you.
             or (statevalue is None)  # or you.
-            or (FastFalling and statevalue < self.bermuda_last_state)  # (like Distance)
-            or (FastRising and statevalue > self.bermuda_last_state)  # (like RSSI)
+            or (
+                fast_falling and statevalue < self.bermuda_last_state
+            )  # (like Distance)
+            or (fast_rising and statevalue > self.bermuda_last_state)  # (like RSSI)
         ):
             # Publish the new value and update cache
             self.bermuda_last_stamp = nowstamp

custom_components/bermuda/sensor.py

@@ -171,7 +171,7 @@ def name(self):
     @property
     def native_value(self):
         return self._cached_ratelimit(
-            self._device.area_rssi, FastFalling=False, FastRising=True
+            self._device.area_rssi, fast_falling=False, fast_rising=True
         )
 
     @property