Skip to content

Commit

Permalink
Make Reading a data class
Browse files Browse the repository at this point in the history
Amongst other advantages (such as repr), this makes it implement
`__eq__`, allowing it to be compared in unit tests.

While a reading ought to typically be immutable, I didn't make it frozen
since I don't want to break backwards compatibility for any code that
does modify a reading.

See NGC-1063.
  • Loading branch information
bmerry committed Sep 18, 2023
1 parent 3984b77 commit 26321e6
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 39 deletions.
11 changes: 6 additions & 5 deletions src/aiokatcp/sensor.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import warnings
import weakref
from abc import ABCMeta, abstractmethod
from dataclasses import dataclass
from typing import (
Any,
Callable,
Expand Down Expand Up @@ -79,6 +80,7 @@ def __call__(self, __sensor: "Sensor[_T]", __reading: "Reading[_T]") -> None:
Observer = Union[ClassicObserver[_T], DeltaObserver[_T]]


@dataclass
class Reading(Generic[_T]):
"""Sensor reading
Expand All @@ -92,12 +94,11 @@ class Reading(Generic[_T]):
Sensor value at `timestamp`
"""

# Once Python 3.10 is the minimum, pass 'slots' parameter to dataclass instead
__slots__ = ("timestamp", "status", "value")

def __init__(self, timestamp: float, status: "Sensor.Status", value: _T) -> None:
self.timestamp = timestamp
self.status = status
self.value = value
timestamp: float
status: "Sensor.Status"
value: _T


def _default_status_func(value) -> "Sensor.Status":
Expand Down
9 changes: 3 additions & 6 deletions tests/test_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
InvalidReply,
Message,
ProtocolError,
Reading,
Sensor,
SensorWatcher,
SyncState,
Expand Down Expand Up @@ -734,14 +735,10 @@ def test_sensor_updated(self, watcher: DummySensorWatcher) -> None:
watcher.sensor_updated("bar", b"42", Sensor.Status.ERROR, 1234567891.5)
watcher.batch_stop()
sensor = watcher.sensors["test_foo"]
assert sensor.value == 12.5
assert sensor.status == Sensor.Status.WARN
assert sensor.timestamp == 1234567890.0
assert sensor.reading == Reading(1234567890.0, Sensor.Status.WARN, 12.5)
for name in ["test_bar1", "test_bar2"]:
sensor = watcher.sensors[name]
assert sensor.value == 42
assert sensor.status == Sensor.Status.ERROR
assert sensor.timestamp == 1234567891.5
assert sensor.reading == Reading(1234567891.5, Sensor.Status.ERROR, 42)

def test_sensor_updated_bad_value(self, watcher: DummySensorWatcher) -> None:
self.test_sensor_added(watcher)
Expand Down
39 changes: 11 additions & 28 deletions tests/test_sensor.py
Original file line number Diff line number Diff line change
Expand Up @@ -353,9 +353,7 @@ def test_creation(self, agg_sensor, ss, sensors):
"""Check that creation happens properly, and correct initial value is set."""
agg_sensor.update_aggregate.assert_called_with(agg_sensor, None, None, None)
assert agg_sensor.target is ss
assert agg_sensor.reading.timestamp == 0
assert agg_sensor.reading.status == Sensor.Status.NOMINAL
assert agg_sensor.reading.value == 7
assert agg_sensor.reading == Reading(0, Sensor.Status.NOMINAL, 7)

def test_sensor_added(self, agg_sensor, ss, sensors):
"""Check that the update function is called when a sensor is added."""
Expand All @@ -364,9 +362,7 @@ def test_sensor_added(self, agg_sensor, ss, sensors):
agg_sensor.update_aggregate.assert_called_with(
agg_sensor, sensors[1], sensors[1].reading, None
)
assert agg_sensor.reading.timestamp == 7
assert agg_sensor.reading.status == Sensor.Status.WARN
assert agg_sensor.reading.value == 42
assert agg_sensor.reading == Reading(7, Sensor.Status.WARN, 42)

def test_sensor_removed(self, agg_sensor, ss, sensors):
"""Check that the update function is called for a removed sensor."""
Expand Down Expand Up @@ -481,50 +477,37 @@ def agg(self, mock_time, sensors: List[Sensor], ss: SensorSet) -> MySimpleAgg:
return MySimpleAgg(ss, int, "simple-agg", "Test sensor")

def test_initial_state(self, agg: MySimpleAgg, mock_time: mock.Mock) -> None:
assert agg.value == 3
assert agg.timestamp == mock_time.return_value
assert agg.status == Sensor.Status.NOMINAL
assert agg.reading == Reading(mock_time.return_value, Sensor.Status.NOMINAL, 3)

def test_update_sensor(self, agg: MySimpleAgg, sensors: List[Sensor]) -> None:
sensors[0].set_value(11, timestamp=1234512346.0)
assert agg.value == 11
assert agg.timestamp == 1234567890.0 # Time must not go backwards
assert agg.status == Sensor.Status.WARN
# Time must not go backwards
assert agg.reading == Reading(1234567890.0, Sensor.Status.WARN, 11)
# Set to invalid state - should effectively remove the reading
sensors[0].set_value(12, timestamp=1400567891.0, status=Sensor.Status.UNKNOWN)
assert agg.value == 0
assert agg.timestamp == 1400567891.0
assert agg.status == Sensor.Status.NOMINAL
assert agg.reading == Reading(1400567891.0, Sensor.Status.NOMINAL, 0)
# Return to a valid state
sensors[0].set_value(5, timestamp=1400567892.0)
assert agg.value == 5
assert agg.timestamp == 1400567892.0
assert agg.status == Sensor.Status.NOMINAL
assert agg.reading == Reading(1400567892.0, Sensor.Status.NOMINAL, 5)

def test_add_remove_sensor(
self, agg: MySimpleAgg, sensors: List[Sensor], ss: SensorSet, mock_time: mock.Mock
) -> None:
mock_time.return_value = 1234567891.5
sensors[1].set_value(8, timestamp=1400567890.0)
ss.add(sensors[1])
assert agg.value == 11
assert agg.timestamp == mock_time.return_value
assert agg.status == Sensor.Status.WARN
assert agg.reading == Reading(mock_time.return_value, Sensor.Status.WARN, 11)

mock_time.return_value = 1234567892.0
ss.remove(sensors[0])
assert agg.value == 8
assert agg.timestamp == mock_time.return_value
assert agg.status == Sensor.Status.NOMINAL
assert agg.reading == Reading(mock_time.return_value, Sensor.Status.NOMINAL, 8)

def test_filter_updates(self, agg: MySimpleAgg, sensors: List[Sensor]) -> None:
sensors[0].set_value(5, timestamp=1400567890.0, status=Sensor.Status.UNKNOWN)
assert agg.value == 0
assert agg.timestamp == 1400567890.0
assert agg.reading == Reading(1400567890.0, Sensor.Status.NOMINAL, 0)
# Make an update that doesn't change anything
callback = mock.Mock()
agg.attach(callback)
sensors[0].set_value(6, timestamp=1400567891.0, status=Sensor.Status.UNKNOWN)
assert agg.value == 0
assert agg.timestamp == 1400567890.0
assert agg.reading == Reading(1400567890.0, Sensor.Status.NOMINAL, 0)
callback.assert_not_called()

0 comments on commit 26321e6

Please sign in to comment.