Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimize calculate_data and redact_data() #361

Merged
merged 38 commits into from
Dec 1, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
97fae83
some calc data updates
Lash-L Nov 7, 2024
ee32807
Optimize redact_data()
Lash-L Nov 7, 2024
6dfc794
Merge branch 'main' into optimizations
Lash-L Nov 7, 2024
8eab1f0
Merge in dev
Lash-L Nov 7, 2024
ef2c9ab
Clean up and remove self.hist_dist_count
Lash-L Nov 8, 2024
4a0fa09
Some MR comments
Lash-L Nov 11, 2024
0dd4552
Apply suggestions from code review
Lash-L Nov 11, 2024
e6b413f
Address more comments
Lash-L Nov 11, 2024
20c0e0f
change time to 8 hours
Lash-L Nov 11, 2024
da6e06e
Add cancel on shutdown
Lash-L Nov 11, 2024
d82f246
cancel on unload
Lash-L Nov 11, 2024
c29f1fe
Add on unload
Lash-L Nov 11, 2024
19b54c9
Move to init so it only gets run after startup
Lash-L Nov 11, 2024
15dc185
Move to init so it only gets run after startup
Lash-L Nov 11, 2024
05c8512
Try again
Lash-L Nov 11, 2024
7625a63
add back removed
Lash-L Nov 11, 2024
e70e01d
change to async_remove
Lash-L Nov 11, 2024
46e5156
Try try again
Lash-L Nov 11, 2024
aa568bb
Try try again
Lash-L Nov 11, 2024
f9340c7
I think finally fixed
Lash-L Nov 11, 2024
2ddaef0
change error
Lash-L Nov 11, 2024
b8acc4a
Change error again
Lash-L Nov 11, 2024
7b746c9
Change error again
Lash-L Nov 11, 2024
4371094
Merge branch 'main' into optimizations
agittins Nov 13, 2024
2c2e135
fix: regression detecting local usb adaptor as scanner
agittins Nov 13, 2024
10d9d3c
Fix: Global sensor update rate and refpower calibration update (#383)
agittins Nov 13, 2024
9157901
fix: daft typo
agittins Nov 13, 2024
aba9257
fix: Local USB Bluetooth detection (#387) fixes #386
agittins Nov 14, 2024
dc6637f
Add Dutch translation and reduce hardcoded language in scripts (#398)
createthisnl Dec 1, 2024
9ad61b1
some calc data updates
Lash-L Nov 7, 2024
26296a2
Clean up and remove self.hist_dist_count
Lash-L Nov 8, 2024
c29b409
Merge branch 'main' into optimizations
agittins Dec 1, 2024
7f7c652
feat: Redaction improvements
agittins Dec 1, 2024
92d4b45
perf: add lru_cache to rssi cals and clean_charbuf
agittins Dec 1, 2024
07dd6dc
linting
agittins Dec 1, 2024
f565d20
Log redact timing in dump_devices
agittins Dec 1, 2024
6b062be
Cleanups: BermudaDeviceScanner scanner
agittins Dec 1, 2024
f64e492
linting
agittins Dec 1, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions custom_components/bermuda/bermuda_device.py
Original file line number Diff line number Diff line change
Expand Up @@ -249,3 +249,7 @@ def to_dict(self):
val = scanout # noqa
out[var] = val
return out

def __repr__(self) -> str:
"""Help debug devices and figure out what device it is at a glance."""
return self.prefname
119 changes: 62 additions & 57 deletions custom_components/bermuda/bermuda_device_scanner.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,11 @@

from typing import TYPE_CHECKING, cast

from homeassistant.components.bluetooth import MONOTONIC_TIME, BluetoothScannerDevice
from homeassistant.components.bluetooth import (
MONOTONIC_TIME,
BaseHaRemoteScanner,
BluetoothScannerDevice,
)

from .const import (
_LOGGER,
Expand Down Expand Up @@ -85,6 +89,12 @@ def __init__(
self.hist_distance_by_interval = [] # updated per-interval
self.hist_interval = [] # WARNING: This is actually "age of ad when we polled"
self.hist_velocity = [] # Effective velocity versus previous stamped reading
self.cached_remote_scanners = set()
self.rssi_offset = self.options.get(CONF_RSSI_OFFSETS, {}).get(self.address, 0)
self.ref_power = self.options.get(CONF_REF_POWER)
self.attenuation = self.options.get(CONF_ATTENUATION)
self.max_velocity = self.options.get(CONF_MAX_VELOCITY)
self.smoothing_samples = self.options.get(CONF_SMOOTHING_SAMPLES)
agittins marked this conversation as resolved.
Show resolved Hide resolved
self.adverts: dict[str, list] = {
"manufacturer_data": [],
"service_data": [],
Expand All @@ -105,19 +115,20 @@ def update_advertisement(self, scandata: BluetoothScannerDevice):
claims to have data.
"""
# In case the scanner has changed it's details since startup:
self.name = scandata.scanner.name
self.area_id = self.scanner_device.area_id
scanner = scandata.scanner
self.name: str = scanner.name
self.area_id: str = self.scanner_device.area_id
self.area_name = self.scanner_device.area_name
new_stamp = None

new_stamp: float | None
# Only remote scanners log timestamps here (local usb adaptors do not),
if hasattr(scandata.scanner, "_discovered_device_timestamps"):
if scanner.source in self.cached_remote_scanners or isinstance(scanner, BaseHaRemoteScanner):
self.cached_remote_scanners.add(scanner.source)
agittins marked this conversation as resolved.
Show resolved Hide resolved
# Found a remote scanner which has timestamp history...
self.scanner_sends_stamps = True
# There's no API for this, so we somewhat sneakily are accessing
# what is intended to be a protected dict.
# pylint: disable-next=protected-access
stamps = scandata.scanner._discovered_device_timestamps # type: ignore #noqa
stamps = scanner._discovered_device_timestamps # type: ignore #noqa

# In this dict all MAC address keys are upper-cased
uppermac = self.parent_device_address.upper()
Expand All @@ -133,7 +144,7 @@ def update_advertisement(self, scandata: BluetoothScannerDevice):
# of this scanner if it hadn't seen this device.
_LOGGER.error(
"Scanner %s has no stamp for %s - very odd.",
scandata.scanner.source,
scanner.source,
self.parent_device_address,
)
new_stamp = None
Expand Down Expand Up @@ -184,8 +195,8 @@ def update_advertisement(self, scandata: BluetoothScannerDevice):

# Safe to update these values regardless of stamps...

self.adapter: str = scandata.scanner.adapter
self.source: str = scandata.scanner.source
self.adapter: str = scanner.adapter
self.source: str = scanner.source
if self.tx_power is not None and scandata.advertisement.tx_power != self.tx_power:
# Not really an erorr, we just don't account for this happening -
# I want to know if it does.
Expand Down Expand Up @@ -306,14 +317,14 @@ def calculate_data(self):
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:
if self.rssi_distance is None and new_stamp is not None:
# DEVICE HAS ARRIVED!
# We have just newly come into range (or we're starting up)
# accept the new reading as-is.
self.rssi_distance = self.rssi_distance_raw
# And ensure the smoothing history gets a fresh start
self.hist_distance_by_interval.insert(0, self.rssi_distance_raw)
del self.hist_distance_by_interval[1:]

self.hist_distance_by_interval = [self.rssi_distance_raw]
Lash-L marked this conversation as resolved.
Show resolved Hide resolved

elif new_stamp is None and (self.stamp is None or self.stamp < MONOTONIC_TIME() - DISTANCE_TIMEOUT):
# DEVICE IS AWAY!
Expand All @@ -338,34 +349,28 @@ def calculate_data(self):
peak_velocity = 0
# walk through the history of distances/stamps, and find
# the peak
for i, old_distance in enumerate(self.hist_distance):
if i == 0:
# (skip the first entry since it's what we're comparing with)
continue

if self.hist_stamp[i] is None:
continue # Skip this iteration if hist_stamp[i] is None

delta_t = velo_newstamp - self.hist_stamp[i]
delta_d = velo_newdistance - old_distance
if delta_t <= 0:
# Additionally, skip if delta_t is zero or negative
# to avoid division by zero
continue

velocity = delta_d / delta_t

# Approach velocities are only interesting vs the previous
# reading, while retreats need to be sensible over time
if i == 1:
# on first round we want approach or retreat velocity
peak_velocity = velocity
if velocity < 0:
# if our new reading is an approach, we are done here
# (not so for == 0 since it might still be an invalid retreat)
break

peak_velocity = max(velocity, peak_velocity)
delta_t = velo_newstamp - self.hist_stamp[1]
delta_d = velo_newdistance - self.hist_distance[1]
if delta_t > 0:
peak_velocity = delta_d / delta_t
# if our initial reading is an approach, we are done here
if peak_velocity >= 0:
for old_distance, hist_stamp in zip(self.hist_distance[2:], self.hist_stamp[2:], strict=False):
if hist_stamp is None:
continue # Skip this iteration if hist_stamp[i] is None

delta_t = velo_newstamp - hist_stamp
if delta_t <= 0:
# Additionally, skip if delta_t is zero or negative
# to avoid division by zero
continue
delta_d = velo_newdistance - old_distance

velocity = delta_d / delta_t

if velocity > peak_velocity:
# but on subsequent comparisons we only care if they're faster retreats
peak_velocity = velocity
Lash-L marked this conversation as resolved.
Show resolved Hide resolved
# we've been through the history and have peak velo retreat, or the most recent
# approach velo.
velocity = peak_velocity
Expand All @@ -375,7 +380,7 @@ def calculate_data(self):

self.hist_velocity.insert(0, velocity)

if velocity > self.options.get(CONF_MAX_VELOCITY):
if velocity > self.max_velocity:
if self.parent_device_address.upper() in self.options.get(CONF_DEVICES, []):
_LOGGER.debug(
"This sparrow %s flies too fast (%2fm/s), ignoring",
Expand All @@ -387,9 +392,13 @@ def calculate_data(self):
else:
# Looks valid enough, add the current reading to the interval log
self.hist_distance_by_interval.insert(0, self.rssi_distance_raw)
dist_count = len(self.hist_distance_by_interval)

# trim the log to length
del self.hist_distance_by_interval[self.options.get(CONF_SMOOTHING_SAMPLES) :]
if self.smoothing_samples < dist_count:
del self.hist_distance_by_interval[self.smoothing_samples :]
# It should only ever need to remove one
dist_count -= 1
Copy link
Owner

Choose a reason for hiding this comment

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

should :-) ... this makes me a little uneasy due to the risk of me introducing a bug later!

Is this worth the opt in the current setup? I wouldn't be surprised if del already does a len() check to short-circuit the operation, so this whole block might be slower.

Would also be good to be rid of dist_count here so that the (potential) use later is less confusing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As far as I am aware, del does not do a len check. And arithmetic operation should be faster than deleting nothing.

That being said, I assume most devices will hit the smoothing samples number and therefore always have to delete something, so maybe it makes sense to remove the if check.


# Calculate a moving-window average, that only includes
# historical values if their "closer" (ie more reliable).
Lash-L marked this conversation as resolved.
Show resolved Hide resolved
Expand All @@ -401,36 +410,28 @@ def calculate_data(self):
# helpful, but probably dependent on use-case.
#
dist_total: float = 0
dist_count: int = 0
local_min: float = self.rssi_distance_raw or DISTANCE_INFINITE
for distance in self.hist_distance_by_interval:
if distance <= local_min:
dist_total += distance
local_min = distance
else:
dist_total += local_min
dist_count += 1
dist_total += local_min
agittins marked this conversation as resolved.
Show resolved Hide resolved

if dist_count > 0:
movavg = dist_total / dist_count
else:
movavg = local_min
Lash-L marked this conversation as resolved.
Show resolved Hide resolved

# The average is only helpful if it's lower than the actual reading.
if self.rssi_distance_raw is None or movavg < self.rssi_distance_raw:
self.rssi_distance = movavg
else:
self.rssi_distance = self.rssi_distance_raw

# Trim our history lists
for histlist in (
self.hist_distance,
self.hist_interval,
self.hist_rssi,
self.hist_stamp,
self.hist_velocity,
):
del histlist[HIST_KEEP_COUNT:]
del self.hist_distance[HIST_KEEP_COUNT:]
del self.hist_interval[HIST_KEEP_COUNT:]
del self.hist_rssi[HIST_KEEP_COUNT:]
del self.hist_stamp[HIST_KEEP_COUNT:]
del self.hist_velocity[HIST_KEEP_COUNT:]

def to_dict(self):
"""Convert class to serialisable dict for dump_devices."""
Expand All @@ -454,3 +455,7 @@ def to_dict(self):
continue
out[var] = val
return out

def __repr__(self) -> str:
"""Help debugging by giving it a clear name instead of empty dict."""
return self.address
agittins marked this conversation as resolved.
Show resolved Hide resolved
14 changes: 12 additions & 2 deletions custom_components/bermuda/coordinator.py
Original file line number Diff line number Diff line change
Expand Up @@ -1282,8 +1282,18 @@ def redact_data(self, data):
self.redaction_list_update()
if isinstance(data, str):
# the end of the recursive wormhole, do the actual work:
for find, fix in self.redactions.items():
data = re.sub(find, fix, data, flags=re.IGNORECASE)
if ":" in data:
if data not in self.redactions:
if data.upper() not in self.redactions:
Lash-L marked this conversation as resolved.
Show resolved Hide resolved
for find, fix in list(self.redactions.items()):
if find in data:
self.redactions[data] = re.sub(find, fix, data, flags=re.IGNORECASE)
data = self.redactions[data]
break
else:
data = self.redactions[data.upper()]
else:
data = self.redactions[data]
# redactions done, now replace any remaining MAC addresses
# We are only looking for xx:xx:xx... format.
return self._redact_generic_re.sub(self._redact_generic_sub, data)
Expand Down
Loading