From 72541bfe44eb76fcf4c3f33d8294833302024509 Mon Sep 17 00:00:00 2001 From: Walter Boring Date: Wed, 23 Sep 2026 22:45:55 -0400 Subject: [PATCH 1/3] refactor: self-registering packet registry in packets/core Replace the get_packet_type() if/elif chain and static TYPE_LOOKUP with a self-registering packet registry. Each concrete Packet subclass declares a packet_type ClassVar and a matches() classifier; __init_subclass__ registers it automatically and TYPE_LOOKUP is derived from the registry so the two can no longer drift out of sync. - factory() now returns Packet | UnknownPacket (was wrongly typed as type[Packet]); _type deserialization uses a registry-backed name lookup instead of globals() - ThirdPartyPacket.subpacket is typed Optional[Packet] and from_dict() re-dispatches the raw sub-packet through factory() so dataclasses-json doesn't pre-build a lossy base Packet - beacon fallback (unrecognized format + latitude) preserved in get_packet_type(); dead factory branches removed Behavior is unchanged: full test suite (775) passes. --- aprsd/packets/core.py | 209 +++++++++++++++++++++++++----------------- 1 file changed, 124 insertions(+), 85 deletions(-) diff --git a/aprsd/packets/core.py b/aprsd/packets/core.py index 23d7982a..59a9f0bc 100644 --- a/aprsd/packets/core.py +++ b/aprsd/packets/core.py @@ -5,7 +5,7 @@ from datetime import datetime, timezone # Due to a failure in python 3.8 -from typing import Any, List, Optional, Type, TypeVar, Union +from typing import Any, ClassVar, List, Optional, Type, TypeVar, Union from aprslib import util as aprslib_util from dataclasses_json import ( @@ -78,9 +78,43 @@ def _translate_fields(raw: dict) -> dict: return raw +# Ordered list of packet classes used to dispatch a raw aprslib dict to the +# right packet type. Every concrete Packet subclass auto-registers here via +# __init_subclass__; the catch-all UnknownPacket is appended explicitly at the +# bottom of the module so it is always evaluated last. +_REGISTRY: list[type['Packet']] = [] + + +def register_packet_type(cls: type) -> type: + """Explicitly register a packet class (used for non-Packet catch-alls).""" + _REGISTRY.append(cls) + return cls + + @dataclass_json @dataclass(unsafe_hash=True) class Packet: + # The APRS category string this packet class represents (e.g. 'message'). + # Empty for abstract base classes. Concrete subclasses set this and are + # automatically registered for factory() dispatch. + packet_type: ClassVar[str] = '' + + def __init_subclass__(cls, **kwargs): + super().__init_subclass__(**kwargs) + if cls.packet_type: + _REGISTRY.append(cls) + + @classmethod + def matches(cls, raw: dict) -> bool: + """Return True if this packet class handles the raw aprslib dict. + + The default implementation matches on the 'format' key. Subclasses + with a more specific classifier (e.g. ack vs reject vs message, or + weather detection) override this so the registry lookup is independent + of class definition order. + """ + return raw.get('format') == cls.packet_type + _type: str = field(default='Packet', hash=False) from_call: Optional[str] = field(default=None) to_call: Optional[str] = field(default=None) @@ -196,6 +230,11 @@ def __repr__(self) -> str: @dataclass(unsafe_hash=True) class AckPacket(Packet): _type: str = field(default='AckPacket', hash=False) + packet_type: ClassVar[str] = PACKET_TYPE_ACK + + @classmethod + def matches(cls, raw: dict) -> bool: + return raw.get('format') == PACKET_TYPE_MESSAGE and raw.get('response') == 'ack' def _build_payload(self): self.payload = f':{self.to_call: <9}:ack{self.msgNo}' @@ -205,6 +244,7 @@ def _build_payload(self): @dataclass(unsafe_hash=True) class BulletinPacket(Packet): _type: str = 'BulletinPacket' + packet_type: ClassVar[str] = PACKET_TYPE_BULLETIN # Holds the encapsulated packet bid: Optional[str] = field(default='1') message_text: Optional[str] = field(default=None) @@ -226,8 +266,13 @@ def _build_payload(self) -> None: @dataclass(unsafe_hash=True) class RejectPacket(Packet): _type: str = field(default='RejectPacket', hash=False) + packet_type: ClassVar[str] = PACKET_TYPE_REJECT response: Optional[str] = field(default=None) + @classmethod + def matches(cls, raw: dict) -> bool: + return raw.get('format') == PACKET_TYPE_MESSAGE and raw.get('response') == 'rej' + def __post_init__(self): if self.response: LOG.warning('Response set!') @@ -240,8 +285,17 @@ def _build_payload(self): @dataclass(unsafe_hash=True) class MessagePacket(Packet): _type: str = field(default='MessagePacket', hash=False) + packet_type: ClassVar[str] = PACKET_TYPE_MESSAGE message_text: Optional[str] = field(default=None) + @classmethod + def matches(cls, raw: dict) -> bool: + # ack/reject are more specific classifiers and must take precedence + return raw.get('format') == PACKET_TYPE_MESSAGE and raw.get('response') not in ( + 'ack', + 'rej', + ) + @property def human_info(self) -> str: self.prepare() @@ -274,6 +328,7 @@ def _build_payload(self): @dataclass(unsafe_hash=True) class StatusPacket(Packet): _type: str = field(default='StatusPacket', hash=False) + packet_type: ClassVar[str] = PACKET_TYPE_STATUS status: Optional[str] = field(default=None) messagecapable: bool = field(default=False) comment: Optional[str] = field(default=None) @@ -373,6 +428,7 @@ def human_info(self) -> str: @dataclass(unsafe_hash=True) class BeaconPacket(GPSPacket): _type: str = field(default='BeaconPacket', hash=False) + packet_type: ClassVar[str] = PACKET_TYPE_BEACON def _build_payload(self): """The payload is the non headers portion of the packet.""" @@ -412,6 +468,7 @@ def human_info(self) -> str: @dataclass(unsafe_hash=True) class MicEPacket(GPSPacket): _type: str = field(default='MicEPacket', hash=False) + packet_type: ClassVar[str] = PACKET_TYPE_MICE messagecapable: bool = False mbits: Optional[str] = None mtype: Optional[str] = None @@ -436,6 +493,7 @@ def human_info(self) -> str: @dataclass(unsafe_hash=True) class TelemetryPacket(GPSPacket): _type: str = field(default='TelemetryPacket', hash=False) + packet_type: ClassVar[str] = PACKET_TYPE_TELEMETRY messagecapable: bool = False mbits: Optional[str] = None mtype: Optional[str] = None @@ -465,6 +523,7 @@ def human_info(self) -> str: @dataclass(unsafe_hash=True) class ObjectPacket(GPSPacket): _type: str = field(default='ObjectPacket', hash=False) + packet_type: ClassVar[str] = PACKET_TYPE_OBJECT alive: bool = True raw_timestamp: Optional[str] = None symbol: str = field(default='r') @@ -473,6 +532,11 @@ class ObjectPacket(GPSPacket): # 0 to 360 course: int = 0 + @classmethod + def matches(cls, raw: dict) -> bool: + # An object that carries a weather payload is a weather packet + return raw.get('format') == cls.packet_type and 'weather' not in raw + def _build_payload(self): time_zulu = self._build_time_zulu() lat = aprslib_util.latitude_to_ddm(self.latitude) @@ -506,6 +570,7 @@ def human_info(self) -> str: @dataclass(unsafe_hash=True) class WeatherPacket(GPSPacket, DataClassJsonMixin): _type: str = field(default='WeatherPacket', hash=False) + packet_type: ClassVar[str] = PACKET_TYPE_WEATHER symbol: str = '_' wind_speed: float = 0.00 wind_direction: int = 0 @@ -523,6 +588,18 @@ class WeatherPacket(GPSPacket, DataClassJsonMixin): course: Optional[int] = field(default=None) speed: Optional[float] = field(default=None) + @classmethod + def matches(cls, raw: dict) -> bool: + # Weather data arrives via several aprslib 'format' values. + return ( + raw.get('format') == PACKET_TYPE_WX + or ( + raw.get('format') == PACKET_TYPE_UNCOMPRESSED + and raw.get('symbol') == '_' + ) + or (raw.get('format') == PACKET_TYPE_OBJECT and 'weather' in raw) + ) + def _translate(self, raw: dict) -> dict: # aprslib returns the weather data in a 'weather' key # We need to move the data out of the 'weather' key @@ -660,8 +737,9 @@ def _build_raw(self): @dataclass(unsafe_hash=True) class ThirdPartyPacket(Packet, DataClassJsonMixin): _type: str = 'ThirdPartyPacket' + packet_type: ClassVar[str] = PACKET_TYPE_THIRDPARTY # Holds the encapsulated packet - subpacket: Optional[type[Packet]] = field(default=None, compare=True, hash=False) + subpacket: Optional[Packet] = field(default=None, compare=True, hash=False) def __repr__(self): """Build the repr version of the packet.""" @@ -677,7 +755,13 @@ def __repr__(self): @classmethod def from_dict(cls: Type[A], kvs: Json, *, infer_missing=False) -> A: obj = super().from_dict(kvs) - obj.subpacket = factory(obj.subpacket) # type: ignore + sub = kvs.get('subpacket') + if isinstance(sub, dict): + # Disptach the raw sub-packet dict through factory() so the + # concrete subtype (e.g. MessagePacket) is preserved. We use the + # original input rather than obj.subpacket, which dataclasses-json + # may have pre-built as a base Packet. + obj.subpacket = factory(sub) # type: ignore return obj @property @@ -693,6 +777,7 @@ def human_info(self) -> str: @dataclass_json(undefined=Undefined.INCLUDE) @dataclass(unsafe_hash=True) +@register_packet_type class UnknownPacket: """Catchall Packet for things we don't know about. @@ -716,6 +801,11 @@ class UnknownPacket: # Was the packet previously processed (for dupe checking) processed: bool = field(repr=False, default=False, compare=False, hash=False) + @classmethod + def matches(cls, raw: dict) -> bool: + """Catch-all: always matches, so it is evaluated last.""" + return True + @property def key(self) -> str: """Build a key for finding this packet in a dict.""" @@ -726,61 +816,33 @@ def human_info(self) -> str: return str(self.unknown_fields) +# Mapping of packet_type category string -> class, derived from the registry +# so it stays in sync automatically. TYPE_LOOKUP: dict[str, type[Packet]] = { - PACKET_TYPE_BULLETIN: BulletinPacket, - PACKET_TYPE_WX: WeatherPacket, - PACKET_TYPE_WEATHER: WeatherPacket, - PACKET_TYPE_MESSAGE: MessagePacket, - PACKET_TYPE_ACK: AckPacket, - PACKET_TYPE_REJECT: RejectPacket, - PACKET_TYPE_MICE: MicEPacket, - PACKET_TYPE_OBJECT: ObjectPacket, - PACKET_TYPE_STATUS: StatusPacket, - PACKET_TYPE_BEACON: BeaconPacket, - PACKET_TYPE_UNKNOWN: UnknownPacket, - PACKET_TYPE_THIRDPARTY: ThirdPartyPacket, - PACKET_TYPE_TELEMETRY: TelemetryPacket, + cls.packet_type: cls for cls in _REGISTRY if cls.packet_type } +# The catch-all doesn't declare a ClassVar packet_type (it has a runtime +# packet_type field instead), so add it explicitly. +TYPE_LOOKUP[PACKET_TYPE_UNKNOWN] = UnknownPacket def get_packet_type(packet: dict) -> str: - """Decode the packet type from the packet.""" - - pkt_format = packet.get('format') - msg_response = packet.get('response') - packet_type = PACKET_TYPE_UNKNOWN - if pkt_format == 'message' and msg_response == 'ack': - packet_type = PACKET_TYPE_ACK - elif pkt_format == 'message' and msg_response == 'rej': - packet_type = PACKET_TYPE_REJECT - elif pkt_format == 'message': - packet_type = PACKET_TYPE_MESSAGE - elif pkt_format == 'mic-e': - packet_type = PACKET_TYPE_MICE - elif pkt_format == 'object': - packet_type = PACKET_TYPE_OBJECT - elif pkt_format == 'status': - packet_type = PACKET_TYPE_STATUS - elif pkt_format == PACKET_TYPE_BULLETIN: - packet_type = PACKET_TYPE_BULLETIN - elif pkt_format == PACKET_TYPE_BEACON: - packet_type = PACKET_TYPE_BEACON - elif pkt_format == PACKET_TYPE_TELEMETRY: - packet_type = PACKET_TYPE_TELEMETRY - elif pkt_format == PACKET_TYPE_WX: - packet_type = PACKET_TYPE_WEATHER - elif pkt_format == PACKET_TYPE_UNCOMPRESSED: - if packet.get('symbol') == '_': - packet_type = PACKET_TYPE_WEATHER - elif pkt_format == PACKET_TYPE_THIRDPARTY: - packet_type = PACKET_TYPE_THIRDPARTY - - if packet_type == PACKET_TYPE_UNKNOWN: - if 'latitude' in packet: - packet_type = PACKET_TYPE_BEACON - else: - packet_type = PACKET_TYPE_UNKNOWN - return packet_type + """Decode the packet type from the packet. + + Returns the packet_type of the first registered class whose matches() + classifier accepts the raw packet dict. An unrecognized packet that + still carries a position is treated as a beacon (matching the original + get_packet_type() fallback). + """ + for cls in _REGISTRY: + if cls is UnknownPacket: + # The catch-all is evaluated explicitly below + continue + if cls.matches(packet): + return cls.packet_type + if 'latitude' in packet: + return PACKET_TYPE_BEACON + return PACKET_TYPE_UNKNOWN def is_message_packet(packet: dict) -> bool: @@ -796,27 +858,27 @@ def is_mice_packet(packet: dict[Any, Any]) -> bool: # Allowlist of class names that factory() may deserialise from disk. -# Built lazily from TYPE_LOOKUP so it stays in sync automatically. +# Built lazily from the registry so it stays in sync automatically. _KNOWN_PACKET_TYPE_NAMES: set[str] = set() +_PACKET_TYPE_NAME_LOOKUP: dict[str, type] = {} def _known_packet_type_names() -> set[str]: - global _KNOWN_PACKET_TYPE_NAMES + global _KNOWN_PACKET_TYPE_NAMES, _PACKET_TYPE_NAME_LOOKUP if not _KNOWN_PACKET_TYPE_NAMES: - _KNOWN_PACKET_TYPE_NAMES = {cls.__name__ for cls in TYPE_LOOKUP.values()} | { - 'UnknownPacket' - } + _KNOWN_PACKET_TYPE_NAMES = {cls.__name__ for cls in _REGISTRY} + _PACKET_TYPE_NAME_LOOKUP = {cls.__name__: cls for cls in _REGISTRY} return _KNOWN_PACKET_TYPE_NAMES -def factory(raw_packet: dict[Any, Any]) -> type[Packet]: - """Factory method to create a packet from a raw packet string.""" +def factory(raw_packet: dict[Any, Any]) -> Packet | UnknownPacket: + """Factory method to create a packet from a raw packet dict.""" raw = raw_packet if '_type' in raw: type_name = raw['_type'] if type_name not in _known_packet_type_names(): raise ValueError(f'Unknown packet type {type_name!r} in saved data') - cls = globals()[type_name] + cls = _PACKET_TYPE_NAME_LOOKUP[type_name] return cls.from_dict(raw) raw['raw_dict'] = raw.copy() @@ -826,27 +888,4 @@ def factory(raw_packet: dict[Any, Any]) -> type[Packet]: raw['packet_type'] = packet_type packet_class = TYPE_LOOKUP[packet_type] - if packet_type == PACKET_TYPE_WX: - # the weather information is in a dict - # this brings those values out to the outer dict - packet_class = WeatherPacket - elif packet_type == PACKET_TYPE_OBJECT and 'weather' in raw: - packet_class = WeatherPacket - elif packet_type == PACKET_TYPE_UNKNOWN: - # Try and figure it out here - if 'latitude' in raw: - packet_class = GPSPacket - else: - # LOG.warning(raw) - packet_class = UnknownPacket - - raw.get('addresse', raw.get('to_call')) - - # TODO: Find a global way to enable/disable this - # LOGU.opt(colors=True).info( - # f"factory({packet_type: <8}):" - # f"({packet_class.__name__: <13}): " - # f"{raw.get('from_call'): <9} -> {to: <9}") - # LOG.info(raw.get('msgNo')) - - return packet_class().from_dict(raw) # type: ignore + return packet_class().from_dict(raw) From f30f7640acfa6948b9ecd37fe93629ce98886090 Mon Sep 17 00:00:00 2001 From: Walter Boring Date: Wed, 23 Sep 2026 23:06:52 -0400 Subject: [PATCH 2/3] refactor: compose Position value object into GPSPacket Replace the flat GPS fields on GPSPacket and its subclasses with a composed Position value object (has-a instead of is-a), collapsing the Packet -> GPSPacket -> {Beacon, MicE, Telemetry, Object, Weather} data spine. Position owns the coordinates, symbol, movement/PHG data and the conversion/formatting logic (zulu time, DDM, human_info). - GPSPacket holds position: Optional[Position]; subclass defaults preserved via _position_default() (ObjectPacket 'r', WeatherPacket '_') - to_dict()/from_dict() flatten/unflatten so the on-disk JSON format is unchanged; verified via objectstore save/load round-trip - GPSPacket inherits DataClassJsonMixin directly because the @dataclass_json decorator overwrites to_dict()/from_dict() and would clobber the flattening overrides - constructor is breaking: position=Position(...) instead of latitude=.../longitude=...; updated tx.py, log.py and all tests Full test suite (775) passes; ruff format + lint clean. --- aprsd/packets/__init__.py | 1 + aprsd/packets/core.py | 386 ++++++++++++++++++------- aprsd/packets/log.py | 12 +- aprsd/threads/tx.py | 10 +- tests/fake.py | 10 +- tests/packets/test_beacon_packet.py | 50 ++-- tests/packets/test_gps_packet.py | 187 ++++++++++-- tests/packets/test_log.py | 20 +- tests/packets/test_mice_packet.py | 44 +-- tests/packets/test_object_packet.py | 68 +++-- tests/packets/test_telemetry_packet.py | 40 +-- tests/packets/test_tracker.py | 16 +- tests/packets/test_weather_packet.py | 42 +-- tests/test_packets.py | 22 +- 14 files changed, 639 insertions(+), 269 deletions(-) diff --git a/aprsd/packets/__init__.py b/aprsd/packets/__init__.py index b5d854c4..922c71f9 100644 --- a/aprsd/packets/__init__.py +++ b/aprsd/packets/__init__.py @@ -8,6 +8,7 @@ MicEPacket, ObjectPacket, Packet, + Position, RejectPacket, StatusPacket, ThirdPartyPacket, diff --git a/aprsd/packets/core.py b/aprsd/packets/core.py index 59a9f0bc..7efeb2d1 100644 --- a/aprsd/packets/core.py +++ b/aprsd/packets/core.py @@ -1,7 +1,8 @@ import logging import re import time -from dataclasses import dataclass, field +import warnings +from dataclasses import MISSING, dataclass, field, fields, replace from datetime import datetime, timezone # Due to a failure in python 3.8 @@ -348,57 +349,260 @@ def human_info(self) -> str: @dataclass_json -@dataclass(unsafe_hash=True) -class GPSPacket(Packet): +@dataclass(frozen=True) +class Position: + """A station position: coordinates, symbol and movement/PHG data. + + Composed into GPSPacket (has-a) instead of being spread across the GPS + subclass hierarchy (is-a). Keeps the conversion/formatting logic in one + testable value object. + """ + + latitude: float = 0.00 + longitude: float = 0.00 + altitude: float = 0.00 + rng: float = 0.00 + posambiguity: int = 0 + symbol: str = 'l' + symbol_table: str = '/' + comment: Optional[str] = None + course: Optional[int] = None + speed: Optional[float] = None + phg: Optional[str] = None + phg_power: Optional[int] = None + phg_height: Optional[float] = None + phg_gain: Optional[int] = None + phg_dir: Optional[str] = None + phg_range: Optional[float] = None + phg_rate: Optional[int] = None + # http://www.aprs.org/datum.txt + daodatumbyte: Optional[str] = None + + def zulu_time(self, timestamp: float) -> str: + """Build the timestamp in UTC/zulu.""" + return datetime.fromtimestamp(timestamp, tz=timezone.utc).strftime('%d%H%M') + + def ddm_latitude(self) -> str: + return aprslib_util.latitude_to_ddm(self.latitude) + + def ddm_longitude(self) -> str: + return aprslib_util.longitude_to_ddm(self.longitude) + + def human_info(self) -> str: + h_str = [f'Lat:{self.latitude:03.3f}', f'Lon:{self.longitude:03.3f}'] + if self.altitude: + h_str.append(f'Altitude {self.altitude:03.0f}') + if self.speed: + h_str.append(f'Speed {self.speed:03.0f}MPH') + if self.course: + h_str.append(f'Course {self.course:03.0f}') + if self.rng: + h_str.append(f'RNG {self.rng:03.0f}') + if self.phg: + h_str.append(f'PHG {self.phg}') + return ' '.join(h_str) + + +# The flat keyword names that the backward-compat GPSPacket.__init__ accepts +# (see the TODO(remove) note below). +_POSITION_FIELD_NAMES: frozenset[str] = frozenset(Position.__dataclass_fields__) + + +@dataclass(init=False, unsafe_hash=True) +class GPSPacket(Packet, DataClassJsonMixin): + # NB: uses the DataClassJsonMixin base instead of the @dataclass_json + # decorator because the decorator unconditionally overwrites to_dict()/ + # from_dict() with the mixin's versions, which would clobber the + # flattening overrides below. + # + # NB: init=False + the custom __init__ below is a backward-compatibility + # shim. It lets external plugins/extensions keep constructing GPS packet + # types with the legacy flat position keyword arguments + # (latitude=, longitude=, symbol=, comment=, ...) and keep reading flat + # attributes (packet.latitude) even though the position data now lives in + # a composed Position object. TODO(remove): delete the flat-kwargs branch + # of __init__ and the flat attribute properties (and restore plain + # @dataclass on the GPS subclasses) in the next major release once the + # plugin ecosystem has migrated to position=Position(...). _type: str = field(default='GPSPacket', hash=False) - latitude: float = field(default=0.00) - longitude: float = field(default=0.00) - altitude: float = field(default=0.00) - rng: float = field(default=0.00) - posambiguity: int = field(default=0) + position: Optional[Position] = field(default=None, compare=True, hash=True) messagecapable: bool = field(default=False) - comment: Optional[str] = field(default=None) - symbol: str = field(default='l') - symbol_table: str = field(default='/') raw_timestamp: Optional[str] = field(default=None) object_name: Optional[str] = field(default=None) object_format: Optional[str] = field(default=None) alive: Optional[bool] = field(default=None) - course: Optional[int] = field(default=None) - speed: Optional[float] = field(default=None) - phg: Optional[str] = field(default=None) - phg_power: Optional[int] = field(default=None) - phg_height: Optional[float] = field(default=None) - phg_gain: Optional[int] = field(default=None) - phg_dir: Optional[str] = field(default=None) - phg_range: Optional[float] = field(default=None) - phg_rate: Optional[int] = field(default=None) - # http://www.aprs.org/datum.txt - daodatumbyte: Optional[str] = field(default=None) + + def __init__(self, *args, **kwargs): + # TODO(remove): backward-compat shim - see class docstring/comment. + if args: + raise TypeError( + f'{type(self).__name__} only accepts keyword arguments; ' + 'pass position=Position(...) for GPS data.' + ) + pos = kwargs.pop('position', None) + flat = {k: kwargs.pop(k) for k in list(kwargs) if k in _POSITION_FIELD_NAMES} + if pos is not None and flat: + raise TypeError( + 'pass either position=Position(...) or the legacy flat ' + f'position keyword arguments ({", ".join(sorted(flat))}), ' + 'not both' + ) + if pos is None and flat: + # Deprecated legacy constructor form. Build the Position from the + # class's default so per-type symbol conventions (ObjectPacket 'r', + # WeatherPacket '_') are preserved exactly as before. + warnings.warn( + 'Passing flat position keyword arguments (latitude=, ' + 'longitude=, ...) to GPS packet types is deprecated; use ' + 'position=Position(...) instead. This backward-compatibility ' + 'shim will be removed in a future release.', + DeprecationWarning, + stacklevel=2, + ) + pos = replace(self.__class__._position_default(), **flat) + # Apply every dataclass field (including inherited and subclass + # fields): explicit kwargs win, otherwise use the declared default. + for f in fields(self): + if f.name == 'position': + continue + if f.name in kwargs: + setattr(self, f.name, kwargs.pop(f.name)) + elif f.default is not MISSING: + setattr(self, f.name, f.default) + elif f.default_factory is not MISSING: + setattr(self, f.name, f.default_factory()) + if kwargs: + raise TypeError(f'Unexpected keyword argument(s): {sorted(kwargs)}') + self.position = pos + + @classmethod + def _position_default(cls) -> Position: + """The Position used when a packet carries none (e.g. via from_dict).""" + return Position() + + def _position(self) -> Position: + return self.position if self.position is not None else self._position_default() + + # TODO(remove): the flat attribute properties below are a backward-compat + # read shim (see class comment). Once the shim is removed, callers use + # packet.position. instead. + @property + def latitude(self) -> float: + return self._position().latitude + + @property + def longitude(self) -> float: + return self._position().longitude + + @property + def altitude(self) -> float: + return self._position().altitude + + @property + def rng(self) -> float: + return self._position().rng + + @property + def posambiguity(self) -> int: + return self._position().posambiguity + + @property + def symbol(self) -> str: + return self._position().symbol + + @property + def symbol_table(self) -> str: + return self._position().symbol_table + + @property + def comment(self) -> Optional[str]: + return self._position().comment + + @property + def course(self) -> Optional[int]: + return self._position().course + + @property + def speed(self) -> Optional[float]: + return self._position().speed + + @property + def phg(self) -> Optional[str]: + return self._position().phg + + @property + def phg_power(self) -> Optional[int]: + return self._position().phg_power + + @property + def phg_height(self) -> Optional[float]: + return self._position().phg_height + + @property + def phg_gain(self) -> Optional[int]: + return self._position().phg_gain + + @property + def phg_dir(self) -> Optional[str]: + return self._position().phg_dir + + @property + def phg_range(self) -> Optional[float]: + return self._position().phg_range + + @property + def phg_rate(self) -> Optional[int]: + return self._position().phg_rate + + @property + def daodatumbyte(self) -> Optional[str]: + return self._position().daodatumbyte def _build_time_zulu(self): """Build the timestamp in UTC/zulu.""" if self.timestamp: - return datetime.fromtimestamp(self.timestamp, tz=timezone.utc).strftime( - '%d%H%M' - ) + return self._position().zulu_time(self.timestamp) + + @classmethod + def from_dict(cls: Type[A], kvs: Json, *, infer_missing=False) -> A: + """Deserialize a packet from a flat dict. + + Position data lives in a nested Position object, but the JSON/disk + format stays flat (latitude, longitude, symbol, ...) so persisted + data keeps loading. The flat position keys are folded into the + nested 'position' field here. + """ + raw = dict(kvs) + pos_keys = set(Position.__dataclass_fields__) + pos = {k: raw.pop(k) for k in list(raw) if k in pos_keys} + if pos: + raw['position'] = pos + return super().from_dict(raw, infer_missing=infer_missing) + + def to_dict(self, encode_json=False): + """Serialize the packet, flattening position back to top-level keys.""" + d = super().to_dict(encode_json=encode_json) + if isinstance(d, dict): + pos = d.pop('position', None) + if pos: + d.update(pos) + return d def _build_payload(self): """The payload is the non headers portion of the packet.""" time_zulu = self._build_time_zulu() - lat = aprslib_util.latitude_to_ddm(self.latitude) - long = aprslib_util.longitude_to_ddm(self.longitude) + pos = self._position() payload = [ '@' if self.timestamp else '!', time_zulu, - lat, - self.symbol_table, - long, - self.symbol, + pos.ddm_latitude(), + pos.symbol_table, + pos.ddm_longitude(), + pos.symbol, ] - if self.comment: - payload.append(self._filter_for_send(self.comment)) + if pos.comment: + payload.append(self._filter_for_send(pos.comment)) self.payload = ''.join(payload) @@ -407,25 +611,12 @@ def _build_raw(self): @property def human_info(self) -> str: - h_str = [] - h_str.append(f'Lat:{self.latitude:03.3f}') - h_str.append(f'Lon:{self.longitude:03.3f}') - if self.altitude: - h_str.append(f'Altitude {self.altitude:03.0f}') - if self.speed: - h_str.append(f'Speed {self.speed:03.0f}MPH') - if self.course: - h_str.append(f'Course {self.course:03.0f}') - if self.rng: - h_str.append(f'RNG {self.rng:03.0f}') - if self.phg: - h_str.append(f'PHG {self.phg}') - - return ' '.join(h_str) + return self._position().human_info() -@dataclass_json -@dataclass(unsafe_hash=True) +@dataclass( + init=False, unsafe_hash=True +) # TODO(remove): init=False only needed for the backward-compat GPSPacket.__init__ shim class BeaconPacket(GPSPacket): _type: str = field(default='BeaconPacket', hash=False) packet_type: ClassVar[str] = PACKET_TYPE_BEACON @@ -433,16 +624,17 @@ class BeaconPacket(GPSPacket): def _build_payload(self): """The payload is the non headers portion of the packet.""" time_zulu = self._build_time_zulu() - lat = aprslib_util.latitude_to_ddm(self.latitude) - lon = aprslib_util.longitude_to_ddm(self.longitude) + pos = self._position() + lat = pos.ddm_latitude() + lon = pos.ddm_longitude() - self.payload = f'@{time_zulu}z{lat}{self.symbol_table}{lon}' + self.payload = f'@{time_zulu}z{lat}{pos.symbol_table}{lon}' - if self.comment: - comment = self._filter_for_send(self.comment) - self.payload = f'{self.payload}{self.symbol}{comment}' + if pos.comment: + comment = self._filter_for_send(pos.comment) + self.payload = f'{self.payload}{pos.symbol}{comment}' else: - self.payload = f'{self.payload}{self.symbol}APRSD Beacon' + self.payload = f'{self.payload}{pos.symbol}APRSD Beacon' def _build_raw(self): self.raw = f'{self.from_call}>APZ100:{self.payload}' @@ -457,26 +649,24 @@ def key(self) -> str: @property def human_info(self) -> str: - h_str = [] - h_str.append(f'Lat:{self.latitude:03.3f}') - h_str.append(f'Lon:{self.longitude:03.3f}') - h_str.append(f'{self.comment}') + pos = self._position() + h_str = [ + f'Lat:{pos.latitude:03.3f}', + f'Lon:{pos.longitude:03.3f}', + f'{pos.comment}', + ] return ' '.join(h_str) -@dataclass_json -@dataclass(unsafe_hash=True) +@dataclass( + init=False, unsafe_hash=True +) # TODO(remove): see GPSPacket.__init__ compat note class MicEPacket(GPSPacket): _type: str = field(default='MicEPacket', hash=False) packet_type: ClassVar[str] = PACKET_TYPE_MICE - messagecapable: bool = False mbits: Optional[str] = None mtype: Optional[str] = None telemetry: Optional[dict] = field(default=None, hash=False) - # in MPH - speed: float = 0.00 - # 0 to 360 - course: int = 0 @property def key(self) -> str: @@ -489,21 +679,17 @@ def human_info(self) -> str: return f'{h_info} {self.mbits} mbits' -@dataclass_json -@dataclass(unsafe_hash=True) +@dataclass( + init=False, unsafe_hash=True +) # TODO(remove): see GPSPacket.__init__ compat note class TelemetryPacket(GPSPacket): _type: str = field(default='TelemetryPacket', hash=False) packet_type: ClassVar[str] = PACKET_TYPE_TELEMETRY - messagecapable: bool = False mbits: Optional[str] = None mtype: Optional[str] = None telemetry: Optional[dict] = field(default=None) tPARM: Optional[list[str]] = field(default=None, hash=False) # noqa: N815 tUNIT: Optional[list[str]] = field(default=None, hash=False) # noqa: N815 - # in MPH - speed: float = 0.00 - # 0 to 360 - course: int = 0 @property def key(self) -> str: @@ -519,18 +705,18 @@ def human_info(self) -> str: return f'{h_info} {self.telemetry}' -@dataclass_json -@dataclass(unsafe_hash=True) +@dataclass( + init=False, unsafe_hash=True +) # TODO(remove): see GPSPacket.__init__ compat note class ObjectPacket(GPSPacket): _type: str = field(default='ObjectPacket', hash=False) packet_type: ClassVar[str] = PACKET_TYPE_OBJECT alive: bool = True raw_timestamp: Optional[str] = None - symbol: str = field(default='r') - # in MPH - speed: float = 0.00 - # 0 to 360 - course: int = 0 + + @classmethod + def _position_default(cls) -> Position: + return Position(symbol='r') @classmethod def matches(cls, raw: dict) -> bool: @@ -539,13 +725,14 @@ def matches(cls, raw: dict) -> bool: def _build_payload(self): time_zulu = self._build_time_zulu() - lat = aprslib_util.latitude_to_ddm(self.latitude) - long = aprslib_util.longitude_to_ddm(self.longitude) + pos = self._position() + lat = pos.ddm_latitude() + long = pos.ddm_longitude() - self.payload = f'*{time_zulu}z{lat}{self.symbol_table}{long}{self.symbol}' + self.payload = f'*{time_zulu}z{lat}{pos.symbol_table}{long}{pos.symbol}' - if self.comment: - comment = self._filter_for_send(self.comment) + if pos.comment: + comment = self._filter_for_send(pos.comment) self.payload = f'{self.payload}{comment}' def _build_raw(self): @@ -564,14 +751,15 @@ def _build_raw(self): @property def human_info(self) -> str: h_info = super().human_info - return f'{h_info} {self.comment}' + return f'{h_info} {self._position().comment}' -@dataclass(unsafe_hash=True) +@dataclass( + init=False, unsafe_hash=True +) # TODO(remove): see GPSPacket.__init__ compat note class WeatherPacket(GPSPacket, DataClassJsonMixin): _type: str = field(default='WeatherPacket', hash=False) packet_type: ClassVar[str] = PACKET_TYPE_WEATHER - symbol: str = '_' wind_speed: float = 0.00 wind_direction: int = 0 wind_gust: float = 0.00 @@ -582,11 +770,12 @@ class WeatherPacket(GPSPacket, DataClassJsonMixin): rain_since_midnight: float = 0.00 humidity: int = 0 pressure: float = 0.00 - comment: Optional[str] = field(default=None) luminosity: Optional[int] = field(default=None) wx_raw_timestamp: Optional[str] = field(default=None) - course: Optional[int] = field(default=None) - speed: Optional[float] = field(default=None) + + @classmethod + def _position_default(cls) -> Position: + return Position(symbol='_') @classmethod def matches(cls, raw: dict) -> bool: @@ -702,13 +891,14 @@ def _build_payload(self): """ time_zulu = self._build_time_zulu() + pos = self._position() contents = [ - f'@{time_zulu}z{self.latitude}{self.symbol_table}', - f'{self.longitude}{self.symbol}', + f'@{time_zulu}z{pos.latitude}{pos.symbol_table}', + f'{pos.longitude}{pos.symbol}', f'{self.wind_direction:03d}', # Speed = sustained 1 minute wind speed in mph - f'{self.symbol_table}', + f'{pos.symbol_table}', f'{self.wind_speed:03.0f}', # wind gust (peak wind speed in mph in the last 5 minutes) f'g{self.wind_gust:03.0f}', @@ -725,8 +915,8 @@ def _build_payload(self): # Barometric pressure (in tenths of millibars/tenths of hPascal) f'b{self.pressure:05.0f}', ] - if self.comment: - comment = self._filter_for_send(self.comment) + if self._position().comment: + comment = self._filter_for_send(self._position().comment) contents.append(comment) self.payload = ''.join(contents) diff --git a/aprsd/packets/log.py b/aprsd/packets/log.py index 6f33b809..e92926c3 100644 --- a/aprsd/packets/log.py +++ b/aprsd/packets/log.py @@ -168,9 +168,17 @@ def log( logit.append(f'{msg}') # is there distance information? - if isinstance(packet, GPSPacket) and CONF.latitude and CONF.longitude: + if ( + isinstance(packet, GPSPacket) + and packet.position + and CONF.latitude + and CONF.longitude + ): my_coords = (float(CONF.latitude), float(CONF.longitude)) - packet_coords = (float(packet.latitude), float(packet.longitude)) + packet_coords = ( + float(packet.position.latitude), + float(packet.position.longitude), + ) try: bearing = utils.calculate_initial_compass_bearing(my_coords, packet_coords) except Exception as e: diff --git a/aprsd/threads/tx.py b/aprsd/threads/tx.py index c7297699..e7851636 100644 --- a/aprsd/threads/tx.py +++ b/aprsd/threads/tx.py @@ -540,10 +540,12 @@ def loop(self): pkt = core.BeaconPacket( from_call=CONF.callsign, to_call='APRS', - latitude=self.latitude, - longitude=self.longitude, - comment='APRSD GPS Beacon', - symbol=CONF.beacon_symbol, + position=core.Position( + latitude=self.latitude, + longitude=self.longitude, + comment='APRSD GPS Beacon', + symbol=CONF.beacon_symbol, + ), ) try: # Only send it once diff --git a/tests/fake.py b/tests/fake.py index 6c78a823..13135d74 100644 --- a/tests/fake.py +++ b/tests/fake.py @@ -41,10 +41,12 @@ def fake_gps_packet(): packet = core.GPSPacket( from_call=FAKE_FROM_CALLSIGN, to_call=FAKE_TO_CALLSIGN, - latitude=37.7749, - longitude=-122.4194, - symbol='>', - comment='Test GPS comment', + position=core.Position( + latitude=37.7749, + longitude=-122.4194, + symbol='>', + comment='Test GPS comment', + ), ) # Call prepare to build the raw data packet.prepare() diff --git a/tests/packets/test_beacon_packet.py b/tests/packets/test_beacon_packet.py index e49c0d56..5574fa88 100644 --- a/tests/packets/test_beacon_packet.py +++ b/tests/packets/test_beacon_packet.py @@ -15,12 +15,15 @@ def test_beacon_packet_to_json(self): packet = packets.BeaconPacket( from_call=fake.FAKE_FROM_CALLSIGN, to_call=fake.FAKE_TO_CALLSIGN, - latitude=37.7749, - longitude=-122.4194, - symbol='>', - symbol_table='/', - comment='Test beacon comment', + position=packets.Position( + latitude=37.7749, + longitude=-122.4194, + symbol='>', + symbol_table='/', + comment='Test beacon comment', + ), ) + json_str = packet.to_json() self.assertIsInstance(json_str, str) json_dict = json.loads(json_str) @@ -49,33 +52,36 @@ def test_beacon_packet_from_dict(self): self.assertIsInstance(packet, packets.BeaconPacket) self.assertEqual(packet.from_call, fake.FAKE_FROM_CALLSIGN) self.assertEqual(packet.to_call, fake.FAKE_TO_CALLSIGN) - self.assertEqual(packet.latitude, 37.7749) - self.assertEqual(packet.longitude, -122.4194) - self.assertEqual(packet.symbol, '>') - self.assertEqual(packet.symbol_table, '/') - self.assertEqual(packet.comment, 'Test beacon comment') + self.assertEqual(packet.position.latitude, 37.7749) + self.assertEqual(packet.position.longitude, -122.4194) + self.assertEqual(packet.position.symbol, '>') + self.assertEqual(packet.position.symbol_table, '/') + self.assertEqual(packet.position.comment, 'Test beacon comment') def test_beacon_packet_round_trip(self): """Test BeaconPacket round-trip: to_json -> from_dict.""" original = packets.BeaconPacket( from_call=fake.FAKE_FROM_CALLSIGN, to_call=fake.FAKE_TO_CALLSIGN, - latitude=37.7749, - longitude=-122.4194, - symbol='>', - symbol_table='/', - comment='Test beacon comment', + position=packets.Position( + latitude=37.7749, + longitude=-122.4194, + symbol='>', + symbol_table='/', + comment='Test beacon comment', + ), ) + json_str = original.to_json() packet_dict = json.loads(json_str) restored = packets.BeaconPacket.from_dict(packet_dict) self.assertEqual(restored.from_call, original.from_call) self.assertEqual(restored.to_call, original.to_call) - self.assertEqual(restored.latitude, original.latitude) - self.assertEqual(restored.longitude, original.longitude) - self.assertEqual(restored.symbol, original.symbol) - self.assertEqual(restored.symbol_table, original.symbol_table) - self.assertEqual(restored.comment, original.comment) + self.assertEqual(restored.position.latitude, original.position.latitude) + self.assertEqual(restored.position.longitude, original.position.longitude) + self.assertEqual(restored.position.symbol, original.position.symbol) + self.assertEqual(restored.position.symbol_table, original.position.symbol_table) + self.assertEqual(restored.position.comment, original.position.comment) self.assertEqual(restored._type, original._type) def test_beacon_packet_from_raw_string(self): @@ -94,5 +100,5 @@ def test_beacon_packet_from_raw_string(self): restored = packets.factory(json_dict) self.assertIsInstance(restored, packets.BeaconPacket) self.assertEqual(restored.from_call, packet.from_call) - self.assertEqual(restored.latitude, packet.latitude) - self.assertEqual(restored.longitude, packet.longitude) + self.assertEqual(restored.position.latitude, packet.position.latitude) + self.assertEqual(restored.position.longitude, packet.position.longitude) diff --git a/tests/packets/test_gps_packet.py b/tests/packets/test_gps_packet.py index 8af8bbb7..c36025c9 100644 --- a/tests/packets/test_gps_packet.py +++ b/tests/packets/test_gps_packet.py @@ -1,5 +1,6 @@ import json import unittest +import warnings import aprslib @@ -15,13 +16,16 @@ def test_gps_packet_to_json(self): packet = packets.GPSPacket( from_call=fake.FAKE_FROM_CALLSIGN, to_call=fake.FAKE_TO_CALLSIGN, - latitude=37.7749, - longitude=-122.4194, - altitude=100.0, - symbol='>', - symbol_table='/', - comment='Test GPS comment', + position=packets.Position( + latitude=37.7749, + longitude=-122.4194, + altitude=100.0, + symbol='>', + symbol_table='/', + comment='Test GPS comment', + ), ) + json_str = packet.to_json() self.assertIsInstance(json_str, str) json_dict = json.loads(json_str) @@ -52,40 +56,43 @@ def test_gps_packet_from_dict(self): self.assertIsInstance(packet, packets.GPSPacket) self.assertEqual(packet.from_call, fake.FAKE_FROM_CALLSIGN) self.assertEqual(packet.to_call, fake.FAKE_TO_CALLSIGN) - self.assertEqual(packet.latitude, 37.7749) - self.assertEqual(packet.longitude, -122.4194) - self.assertEqual(packet.altitude, 100.0) - self.assertEqual(packet.symbol, '>') - self.assertEqual(packet.symbol_table, '/') - self.assertEqual(packet.comment, 'Test GPS comment') + self.assertEqual(packet.position.latitude, 37.7749) + self.assertEqual(packet.position.longitude, -122.4194) + self.assertEqual(packet.position.altitude, 100.0) + self.assertEqual(packet.position.symbol, '>') + self.assertEqual(packet.position.symbol_table, '/') + self.assertEqual(packet.position.comment, 'Test GPS comment') def test_gps_packet_round_trip(self): """Test GPSPacket round-trip: to_json -> from_dict.""" original = packets.GPSPacket( from_call=fake.FAKE_FROM_CALLSIGN, to_call=fake.FAKE_TO_CALLSIGN, - latitude=37.7749, - longitude=-122.4194, - altitude=100.0, - symbol='>', - symbol_table='/', - comment='Test GPS comment', - speed=25.5, - course=180, + position=packets.Position( + latitude=37.7749, + longitude=-122.4194, + altitude=100.0, + symbol='>', + symbol_table='/', + comment='Test GPS comment', + speed=25.5, + course=180, + ), ) + json_str = original.to_json() packet_dict = json.loads(json_str) restored = packets.GPSPacket.from_dict(packet_dict) self.assertEqual(restored.from_call, original.from_call) self.assertEqual(restored.to_call, original.to_call) - self.assertEqual(restored.latitude, original.latitude) - self.assertEqual(restored.longitude, original.longitude) - self.assertEqual(restored.altitude, original.altitude) - self.assertEqual(restored.symbol, original.symbol) - self.assertEqual(restored.symbol_table, original.symbol_table) - self.assertEqual(restored.comment, original.comment) - self.assertEqual(restored.speed, original.speed) - self.assertEqual(restored.course, original.course) + self.assertEqual(restored.position.latitude, original.position.latitude) + self.assertEqual(restored.position.longitude, original.position.longitude) + self.assertEqual(restored.position.altitude, original.position.altitude) + self.assertEqual(restored.position.symbol, original.position.symbol) + self.assertEqual(restored.position.symbol_table, original.position.symbol_table) + self.assertEqual(restored.position.comment, original.position.comment) + self.assertEqual(restored.position.speed, original.position.speed) + self.assertEqual(restored.position.course, original.position.course) self.assertEqual(restored._type, original._type) def test_gps_packet_from_raw_string(self): @@ -96,7 +103,7 @@ def test_gps_packet_from_raw_string(self): # GPS packets are typically created as BeaconPacket or other types # but we can test if it has GPS data self.assertIsNotNone(packet) - if hasattr(packet, 'latitude') and hasattr(packet, 'longitude'): + if packet.position is not None: # Test to_json json_str = packet.to_json() self.assertIsInstance(json_str, str) @@ -105,5 +112,123 @@ def test_gps_packet_from_raw_string(self): self.assertIn('longitude', json_dict) # Test from_dict round trip restored = packets.factory(json_dict) - self.assertEqual(restored.latitude, packet.latitude) - self.assertEqual(restored.longitude, packet.longitude) + self.assertEqual(restored.position.latitude, packet.position.latitude) + self.assertEqual(restored.position.longitude, packet.position.longitude) + + +class TestGPSPacketBackwardCompat(unittest.TestCase): + """Backward-compat shim: legacy flat position kwargs + flat attr reads. + + The GPS packet types accept BOTH position=Position(...) (canonical) and + the legacy flat keyword arguments (latitude=, longitude=, symbol=, ...), + which emit a DeprecationWarning. Flat attribute reads (packet.latitude) + also keep working. These are temporary shims to avoid breaking external + plugins/extensions and are scheduled for removal. + """ + + def _assert_no_deprecation(self, fn): + with warnings.catch_warnings(): + warnings.simplefilter('error', DeprecationWarning) + fn() + + def test_flat_kwargs_deprecated(self): + """Legacy flat kwargs still construct a packet, with a warning.""" + with self.assertWarns(DeprecationWarning): + pkt = packets.BeaconPacket( + from_call=fake.FAKE_FROM_CALLSIGN, + to_call=fake.FAKE_TO_CALLSIGN, + latitude=37.7749, + longitude=-122.4194, + symbol='>', + comment='Test beacon comment', + ) + self.assertIsNotNone(pkt.position) + self.assertEqual(37.7749, pkt.position.latitude) + self.assertEqual(-122.4194, pkt.position.longitude) + self.assertEqual('>', pkt.position.symbol) + self.assertEqual('Test beacon comment', pkt.position.comment) + + def test_flat_kwargs_preserve_per_type_symbol(self): + """Legacy flat kwargs keep the per-type symbol defaults (r/_/l).""" + with self.assertWarns(DeprecationWarning): + obj = packets.ObjectPacket( + from_call='KFAKE', to_call='KMINE', latitude=1.0, longitude=2.0 + ) + self.assertEqual('r', obj.position.symbol) + with self.assertWarns(DeprecationWarning): + wx = packets.WeatherPacket( + from_call='KFAKE', to_call='KMINE', latitude=1.0, longitude=2.0 + ) + self.assertEqual('_', wx.position.symbol) + with self.assertWarns(DeprecationWarning): + gps = packets.GPSPacket( + from_call='KFAKE', to_call='KMINE', latitude=1.0, longitude=2.0 + ) + self.assertEqual('l', gps.position.symbol) + + def test_flat_attribute_reads(self): + """packet.latitude and friends still work as read-only attributes.""" + pkt = packets.GPSPacket( + from_call='KFAKE', + to_call='KMINE', + position=packets.Position( + latitude=37.7749, + longitude=-122.4194, + altitude=100.0, + symbol='>', + comment='c', + speed=25.5, + course=180, + ), + ) + self.assertEqual(37.7749, pkt.latitude) + self.assertEqual(-122.4194, pkt.longitude) + self.assertEqual(100.0, pkt.altitude) + self.assertEqual('>', pkt.symbol) + self.assertEqual('c', pkt.comment) + self.assertEqual(25.5, pkt.speed) + self.assertEqual(180, pkt.course) + + def test_position_kwarg_no_deprecation(self): + """The canonical position= form must not emit a warning.""" + self._assert_no_deprecation( + lambda: packets.BeaconPacket( + from_call='KFAKE', + to_call='KMINE', + position=packets.Position(latitude=1.0, longitude=2.0), + ) + ) + + def test_both_kwargs_raises(self): + """Passing position= AND flat kwargs is ambiguous and must fail.""" + with self.assertRaises(TypeError): + packets.BeaconPacket( + from_call='KFAKE', + to_call='KMINE', + position=packets.Position(latitude=1.0, longitude=2.0), + latitude=1.0, + ) + + def test_positional_args_raises(self): + """GPS packet types only accept keyword arguments.""" + with self.assertRaises(TypeError): + packets.BeaconPacket('KFAKE') + + def test_flat_and_position_constructors_equal(self): + """A legacy flat-kwarg packet equals its position= equivalent.""" + with self.assertWarns(DeprecationWarning): + flat = packets.ObjectPacket( + from_call='KFAKE', + to_call='KMINE', + latitude=1.0, + longitude=2.0, + comment='c', + ) + pos = packets.ObjectPacket( + from_call='KFAKE', + to_call='KMINE', + position=packets.Position( + latitude=1.0, longitude=2.0, comment='c', symbol='r' + ), + ) + self.assertEqual(flat, pos) diff --git a/tests/packets/test_log.py b/tests/packets/test_log.py index a5bc6437..316dd7ac 100644 --- a/tests/packets/test_log.py +++ b/tests/packets/test_log.py @@ -62,10 +62,12 @@ def test_log_multiline_with_gps_packet(self): packet = packets.GPSPacket( from_call=fake.FAKE_FROM_CALLSIGN, to_call=fake.FAKE_TO_CALLSIGN, - latitude=37.7749, - longitude=-122.4194, - symbol='>', - comment='Test GPS comment', + position=packets.Position( + latitude=37.7749, + longitude=-122.4194, + symbol='>', + comment='Test GPS comment', + ), ) packet.send_count = 2 @@ -145,10 +147,12 @@ def test_log_with_gps_packet_distance(self): packet = packets.GPSPacket( from_call=fake.FAKE_FROM_CALLSIGN, to_call=fake.FAKE_TO_CALLSIGN, - latitude=37.7749, - longitude=-122.4194, - symbol='>', - comment='Test GPS comment', + position=packets.Position( + latitude=37.7749, + longitude=-122.4194, + symbol='>', + comment='Test GPS comment', + ), ) packet.send_count = 2 diff --git a/tests/packets/test_mice_packet.py b/tests/packets/test_mice_packet.py index a640a84d..2d185e83 100644 --- a/tests/packets/test_mice_packet.py +++ b/tests/packets/test_mice_packet.py @@ -15,14 +15,17 @@ def test_mice_packet_to_json(self): packet = packets.MicEPacket( from_call=fake.FAKE_FROM_CALLSIGN, to_call=fake.FAKE_TO_CALLSIGN, - latitude=37.7749, - longitude=-122.4194, - speed=25.5, - course=180, + position=packets.Position( + latitude=37.7749, + longitude=-122.4194, + speed=25.5, + course=180, + ), mbits='test', mtype='test_type', telemetry={'key': 'value'}, ) + json_str = packet.to_json() self.assertIsInstance(json_str, str) json_dict = json.loads(json_str) @@ -54,10 +57,10 @@ def test_mice_packet_from_dict(self): self.assertIsInstance(packet, packets.MicEPacket) self.assertEqual(packet.from_call, fake.FAKE_FROM_CALLSIGN) self.assertEqual(packet.to_call, fake.FAKE_TO_CALLSIGN) - self.assertEqual(packet.latitude, 37.7749) - self.assertEqual(packet.longitude, -122.4194) - self.assertEqual(packet.speed, 25.5) - self.assertEqual(packet.course, 180) + self.assertEqual(packet.position.latitude, 37.7749) + self.assertEqual(packet.position.longitude, -122.4194) + self.assertEqual(packet.position.speed, 25.5) + self.assertEqual(packet.position.course, 180) self.assertEqual(packet.mbits, 'test') self.assertEqual(packet.mtype, 'test_type') @@ -66,23 +69,26 @@ def test_mice_packet_round_trip(self): original = packets.MicEPacket( from_call=fake.FAKE_FROM_CALLSIGN, to_call=fake.FAKE_TO_CALLSIGN, - latitude=37.7749, - longitude=-122.4194, - speed=25.5, - course=180, + position=packets.Position( + latitude=37.7749, + longitude=-122.4194, + speed=25.5, + course=180, + ), mbits='test', mtype='test_type', telemetry={'key': 'value'}, ) + json_str = original.to_json() packet_dict = json.loads(json_str) restored = packets.MicEPacket.from_dict(packet_dict) self.assertEqual(restored.from_call, original.from_call) self.assertEqual(restored.to_call, original.to_call) - self.assertEqual(restored.latitude, original.latitude) - self.assertEqual(restored.longitude, original.longitude) - self.assertEqual(restored.speed, original.speed) - self.assertEqual(restored.course, original.course) + self.assertEqual(restored.position.latitude, original.position.latitude) + self.assertEqual(restored.position.longitude, original.position.longitude) + self.assertEqual(restored.position.speed, original.position.speed) + self.assertEqual(restored.position.course, original.position.course) self.assertEqual(restored.mbits, original.mbits) self.assertEqual(restored.mtype, original.mtype) self.assertEqual(restored._type, original._type) @@ -102,6 +108,6 @@ def test_mice_packet_from_raw_string(self): restored = packets.factory(json_dict) self.assertIsInstance(restored, packets.MicEPacket) self.assertEqual(restored.from_call, packet.from_call) - if hasattr(packet, 'latitude') and packet.latitude: - self.assertEqual(restored.latitude, packet.latitude) - self.assertEqual(restored.longitude, packet.longitude) + if (packet.position is not None) and packet.position.latitude: + self.assertEqual(restored.position.latitude, packet.position.latitude) + self.assertEqual(restored.position.longitude, packet.position.longitude) diff --git a/tests/packets/test_object_packet.py b/tests/packets/test_object_packet.py index 5e0db528..81bef231 100644 --- a/tests/packets/test_object_packet.py +++ b/tests/packets/test_object_packet.py @@ -15,15 +15,18 @@ def test_object_packet_to_json(self): packet = packets.ObjectPacket( from_call=fake.FAKE_FROM_CALLSIGN, to_call=fake.FAKE_TO_CALLSIGN, - latitude=37.7749, - longitude=-122.4194, - symbol='r', - symbol_table='/', - comment='Test object comment', + position=packets.Position( + latitude=37.7749, + longitude=-122.4194, + symbol='r', + symbol_table='/', + comment='Test object comment', + speed=25.5, + course=180, + ), alive=True, - speed=25.5, - course=180, ) + json_str = packet.to_json() self.assertIsInstance(json_str, str) json_dict = json.loads(json_str) @@ -58,42 +61,45 @@ def test_object_packet_from_dict(self): self.assertIsInstance(packet, packets.ObjectPacket) self.assertEqual(packet.from_call, fake.FAKE_FROM_CALLSIGN) self.assertEqual(packet.to_call, fake.FAKE_TO_CALLSIGN) - self.assertEqual(packet.latitude, 37.7749) - self.assertEqual(packet.longitude, -122.4194) - self.assertEqual(packet.symbol, 'r') - self.assertEqual(packet.symbol_table, '/') - self.assertEqual(packet.comment, 'Test object comment') + self.assertEqual(packet.position.latitude, 37.7749) + self.assertEqual(packet.position.longitude, -122.4194) + self.assertEqual(packet.position.symbol, 'r') + self.assertEqual(packet.position.symbol_table, '/') + self.assertEqual(packet.position.comment, 'Test object comment') self.assertEqual(packet.alive, True) - self.assertEqual(packet.speed, 25.5) - self.assertEqual(packet.course, 180) + self.assertEqual(packet.position.speed, 25.5) + self.assertEqual(packet.position.course, 180) def test_object_packet_round_trip(self): """Test ObjectPacket round-trip: to_json -> from_dict.""" original = packets.ObjectPacket( from_call=fake.FAKE_FROM_CALLSIGN, to_call=fake.FAKE_TO_CALLSIGN, - latitude=37.7749, - longitude=-122.4194, - symbol='r', - symbol_table='/', - comment='Test object comment', + position=packets.Position( + latitude=37.7749, + longitude=-122.4194, + symbol='r', + symbol_table='/', + comment='Test object comment', + speed=25.5, + course=180, + ), alive=True, - speed=25.5, - course=180, ) + json_str = original.to_json() packet_dict = json.loads(json_str) restored = packets.ObjectPacket.from_dict(packet_dict) self.assertEqual(restored.from_call, original.from_call) self.assertEqual(restored.to_call, original.to_call) - self.assertEqual(restored.latitude, original.latitude) - self.assertEqual(restored.longitude, original.longitude) - self.assertEqual(restored.symbol, original.symbol) - self.assertEqual(restored.symbol_table, original.symbol_table) - self.assertEqual(restored.comment, original.comment) + self.assertEqual(restored.position.latitude, original.position.latitude) + self.assertEqual(restored.position.longitude, original.position.longitude) + self.assertEqual(restored.position.symbol, original.position.symbol) + self.assertEqual(restored.position.symbol_table, original.position.symbol_table) + self.assertEqual(restored.position.comment, original.position.comment) self.assertEqual(restored.alive, original.alive) - self.assertEqual(restored.speed, original.speed) - self.assertEqual(restored.course, original.course) + self.assertEqual(restored.position.speed, original.position.speed) + self.assertEqual(restored.position.course, original.position.course) self.assertEqual(restored._type, original._type) def test_object_packet_from_raw_string(self): @@ -117,6 +123,6 @@ def test_object_packet_from_raw_string(self): self.assertIsInstance(restored, packets.ObjectPacket) self.assertEqual(restored.from_call, packet.from_call) self.assertEqual(restored.to_call, packet.to_call) - if hasattr(packet, 'latitude') and packet.latitude: - self.assertEqual(restored.latitude, packet.latitude) - self.assertEqual(restored.longitude, packet.longitude) + if (packet.position is not None) and packet.position.latitude: + self.assertEqual(restored.position.latitude, packet.position.latitude) + self.assertEqual(restored.position.longitude, packet.position.longitude) diff --git a/tests/packets/test_telemetry_packet.py b/tests/packets/test_telemetry_packet.py index b9b0352b..51e85212 100644 --- a/tests/packets/test_telemetry_packet.py +++ b/tests/packets/test_telemetry_packet.py @@ -4,7 +4,7 @@ import aprslib from aprsd import packets -from aprsd.packets.core import TelemetryPacket +from aprsd.packets.core import Position, TelemetryPacket from tests import fake @@ -16,16 +16,19 @@ def test_telemetry_packet_to_json(self): packet = TelemetryPacket( from_call=fake.FAKE_FROM_CALLSIGN, to_call=fake.FAKE_TO_CALLSIGN, - latitude=37.7749, - longitude=-122.4194, - speed=25.5, - course=180, + position=Position( + latitude=37.7749, + longitude=-122.4194, + speed=25.5, + course=180, + ), mbits='test', mtype='test_type', telemetry={'key': 'value'}, tPARM=['parm1', 'parm2'], tUNIT=['unit1', 'unit2'], ) + json_str = packet.to_json() self.assertIsInstance(json_str, str) json_dict = json.loads(json_str) @@ -59,10 +62,10 @@ def test_telemetry_packet_from_dict(self): self.assertIsInstance(packet, TelemetryPacket) self.assertEqual(packet.from_call, fake.FAKE_FROM_CALLSIGN) self.assertEqual(packet.to_call, fake.FAKE_TO_CALLSIGN) - self.assertEqual(packet.latitude, 37.7749) - self.assertEqual(packet.longitude, -122.4194) - self.assertEqual(packet.speed, 25.5) - self.assertEqual(packet.course, 180) + self.assertEqual(packet.position.latitude, 37.7749) + self.assertEqual(packet.position.longitude, -122.4194) + self.assertEqual(packet.position.speed, 25.5) + self.assertEqual(packet.position.course, 180) self.assertEqual(packet.mbits, 'test') self.assertEqual(packet.mtype, 'test_type') @@ -71,25 +74,28 @@ def test_telemetry_packet_round_trip(self): original = TelemetryPacket( from_call=fake.FAKE_FROM_CALLSIGN, to_call=fake.FAKE_TO_CALLSIGN, - latitude=37.7749, - longitude=-122.4194, - speed=25.5, - course=180, + position=Position( + latitude=37.7749, + longitude=-122.4194, + speed=25.5, + course=180, + ), mbits='test', mtype='test_type', telemetry={'key': 'value'}, tPARM=['parm1', 'parm2'], tUNIT=['unit1', 'unit2'], ) + json_str = original.to_json() packet_dict = json.loads(json_str) restored = TelemetryPacket.from_dict(packet_dict) self.assertEqual(restored.from_call, original.from_call) self.assertEqual(restored.to_call, original.to_call) - self.assertEqual(restored.latitude, original.latitude) - self.assertEqual(restored.longitude, original.longitude) - self.assertEqual(restored.speed, original.speed) - self.assertEqual(restored.course, original.course) + self.assertEqual(restored.position.latitude, original.position.latitude) + self.assertEqual(restored.position.longitude, original.position.longitude) + self.assertEqual(restored.position.speed, original.position.speed) + self.assertEqual(restored.position.course, original.position.course) self.assertEqual(restored.mbits, original.mbits) self.assertEqual(restored.mtype, original.mtype) self.assertEqual(restored._type, original._type) diff --git a/tests/packets/test_tracker.py b/tests/packets/test_tracker.py index 6707c268..0b65b15f 100644 --- a/tests/packets/test_tracker.py +++ b/tests/packets/test_tracker.py @@ -266,9 +266,11 @@ def test_tx_skips_beacon_packet(self): beacon = core.BeaconPacket( from_call='KFAKE', to_call='APRS', - latitude=38.0, - longitude=-121.0, - comment='Test Beacon', + position=core.Position( + latitude=38.0, + longitude=-121.0, + comment='Test Beacon', + ), ) beacon.prepare(create_msg_number=True) initial_total = pt.total_tracked @@ -285,9 +287,11 @@ def test_tx_beacon_not_tracked_even_with_retry_count(self): beacon = core.BeaconPacket( from_call='KFAKE', to_call='APDW16', - latitude=38.0, - longitude=-121.0, - comment='WebChat Beacon', + position=core.Position( + latitude=38.0, + longitude=-121.0, + comment='WebChat Beacon', + ), ) beacon.retry_count = 3 # Even with retries set, don't track beacon.prepare(create_msg_number=True) diff --git a/tests/packets/test_weather_packet.py b/tests/packets/test_weather_packet.py index 0cc34ce9..e37677f2 100644 --- a/tests/packets/test_weather_packet.py +++ b/tests/packets/test_weather_packet.py @@ -15,10 +15,13 @@ def test_weather_packet_to_json(self): packet = packets.WeatherPacket( from_call=fake.FAKE_FROM_CALLSIGN, to_call=fake.FAKE_TO_CALLSIGN, - latitude=37.7749, - longitude=-122.4194, - symbol='_', - symbol_table='/', + position=packets.Position( + latitude=37.7749, + longitude=-122.4194, + symbol='_', + symbol_table='/', + comment='Test weather comment', + ), wind_speed=10.5, wind_direction=180, wind_gust=15.0, @@ -28,8 +31,8 @@ def test_weather_packet_to_json(self): rain_since_midnight=0.3, humidity=65, pressure=1013.25, - comment='Test weather comment', ) + json_str = packet.to_json() self.assertIsInstance(json_str, str) json_dict = json.loads(json_str) @@ -75,9 +78,9 @@ def test_weather_packet_from_dict(self): self.assertIsInstance(packet, packets.WeatherPacket) self.assertEqual(packet.from_call, fake.FAKE_FROM_CALLSIGN) self.assertEqual(packet.to_call, fake.FAKE_TO_CALLSIGN) - self.assertEqual(packet.latitude, 37.7749) - self.assertEqual(packet.longitude, -122.4194) - self.assertEqual(packet.symbol, '_') + self.assertEqual(packet.position.latitude, 37.7749) + self.assertEqual(packet.position.longitude, -122.4194) + self.assertEqual(packet.position.symbol, '_') self.assertEqual(packet.wind_speed, 10.5) self.assertEqual(packet.wind_direction, 180) self.assertEqual(packet.wind_gust, 15.0) @@ -87,17 +90,20 @@ def test_weather_packet_from_dict(self): self.assertEqual(packet.rain_since_midnight, 0.3) self.assertEqual(packet.humidity, 65) self.assertEqual(packet.pressure, 1013.25) - self.assertEqual(packet.comment, 'Test weather comment') + self.assertEqual(packet.position.comment, 'Test weather comment') def test_weather_packet_round_trip(self): """Test WeatherPacket round-trip: to_json -> from_dict.""" original = packets.WeatherPacket( from_call=fake.FAKE_FROM_CALLSIGN, to_call=fake.FAKE_TO_CALLSIGN, - latitude=37.7749, - longitude=-122.4194, - symbol='_', - symbol_table='/', + position=packets.Position( + latitude=37.7749, + longitude=-122.4194, + symbol='_', + symbol_table='/', + comment='Test weather comment', + ), wind_speed=10.5, wind_direction=180, wind_gust=15.0, @@ -107,16 +113,16 @@ def test_weather_packet_round_trip(self): rain_since_midnight=0.3, humidity=65, pressure=1013.25, - comment='Test weather comment', ) + json_str = original.to_json() packet_dict = json.loads(json_str) restored = packets.WeatherPacket.from_dict(packet_dict) self.assertEqual(restored.from_call, original.from_call) self.assertEqual(restored.to_call, original.to_call) - self.assertEqual(restored.latitude, original.latitude) - self.assertEqual(restored.longitude, original.longitude) - self.assertEqual(restored.symbol, original.symbol) + self.assertEqual(restored.position.latitude, original.position.latitude) + self.assertEqual(restored.position.longitude, original.position.longitude) + self.assertEqual(restored.position.symbol, original.position.symbol) self.assertEqual(restored.wind_speed, original.wind_speed) self.assertEqual(restored.wind_direction, original.wind_direction) self.assertEqual(restored.wind_gust, original.wind_gust) @@ -126,7 +132,7 @@ def test_weather_packet_round_trip(self): self.assertEqual(restored.rain_since_midnight, original.rain_since_midnight) self.assertEqual(restored.humidity, original.humidity) self.assertEqual(restored.pressure, original.pressure) - self.assertEqual(restored.comment, original.comment) + self.assertEqual(restored.position.comment, original.position.comment) self.assertEqual(restored._type, original._type) def test_weather_packet_from_raw_string(self): diff --git a/tests/test_packets.py b/tests/test_packets.py index 49e101f0..4b34967e 100644 --- a/tests/test_packets.py +++ b/tests/test_packets.py @@ -151,8 +151,8 @@ def test_weather_factory(self): self.assertEqual(3.0, packet.wind_speed) self.assertEqual(232, packet.wind_direction) self.assertEqual(6.0, packet.wind_gust) - self.assertEqual(29.899, packet.latitude) - self.assertEqual(-84.39616666666667, packet.longitude) + self.assertEqual(29.899, packet.position.latitude) + self.assertEqual(-84.39616666666667, packet.position.longitude) def test_mice_factory(self): packet_raw = 'kh2sr-15>S7TSYR,WIDE1-1,WIDE2-1,qAO,KO6KL-1:`1`7\x1c\x1c.#/`"4,}QuirkyQRP 4.6V 35.3C S06' @@ -204,11 +204,13 @@ def test_beacon_format(self): packet = packets.BeaconPacket( from_call=fake.FAKE_FROM_CALLSIGN, to_call=fake.FAKE_TO_CALLSIGN, - latitude=lat, - longitude=lon, + position=packets.Position( + latitude=lat, + longitude=lon, + symbol='>', + comment=comment, + ), timestamp=ts, - symbol='>', - comment=comment, ) expected_lat = aprslib_util.latitude_to_ddm(lat) @@ -224,10 +226,12 @@ def test_beacon_format_no_comment(self): packet = packets.BeaconPacket( from_call=fake.FAKE_FROM_CALLSIGN, to_call=fake.FAKE_TO_CALLSIGN, - latitude=lat, - longitude=lon, + position=packets.Position( + latitude=lat, + longitude=lon, + symbol='>', + ), timestamp=ts, - symbol='>', ) empty_comment = 'APRSD Beacon' From 03a5fd06c26272ee16cce7a40f4b9ace91e0c4d6 Mon Sep 17 00:00:00 2001 From: Walter Boring Date: Wed, 23 Sep 2026 23:09:23 -0400 Subject: [PATCH 3/3] docs: add Unreleased changelog entries for packet refactors Add an [Unreleased] section to ChangeLog.md covering the self-registering packet registry and the Position composition refactor, and regenerate docs/source/changelog.md via prepare_changelog.py. Also teach prepare_changelog.py to normalize the [Unreleased] heading level so the Sphinx copy stays consistent with version headings. --- ChangeLog.md | 8 ++++++++ docs/prepare_changelog.py | 2 +- docs/source/changelog.md | 8 ++++++++ 3 files changed, 17 insertions(+), 1 deletion(-) diff --git a/ChangeLog.md b/ChangeLog.md index aa1d3ff4..0c667faa 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -4,6 +4,14 @@ All notable changes to this project will be documented in this file. Dates are d Generated by [`auto-changelog`](https://github.com/CookPete/auto-changelog). +#### [Unreleased](https://github.com/craigerl/aprsd/compare/5.0.1...HEAD) + +##### Refactoring + +- refactor: self-registering packet registry in packets/core [`72541bf`](https://github.com/craigerl/aprsd/commit/72541bf) +- refactor: compose Position value object into GPSPacket [`f30f764`](https://github.com/craigerl/aprsd/commit/f30f764) +- Non-breaking: GPS packet types still accept the legacy flat position kwargs (latitude=, longitude=, ...) and flat attribute reads (packet.latitude); the legacy form is deprecated and will be removed in a future release — use position=Position(...) [`f30f764`](https://github.com/craigerl/aprsd/commit/f30f764) + #### [5.0.1](https://github.com/craigerl/aprsd/compare/5.0.0...5.0.1) ##### Bug Fixes diff --git a/docs/prepare_changelog.py b/docs/prepare_changelog.py index 387513c8..96d8d532 100644 --- a/docs/prepare_changelog.py +++ b/docs/prepare_changelog.py @@ -4,7 +4,7 @@ import re from pathlib import Path -VERSION_HEADING = re.compile(r'^#{3,4} (?:\[)?(?:v?\d+\.\d+\.\d+)') +VERSION_HEADING = re.compile(r'^#{3,4} (?:\[)?(?:v?\d+\.\d+\.\d+|Unreleased)') CATEGORY_HEADING = re.compile( r'^#{3,5} (?:Breaking Changes|Features|Bug Fixes|Refactoring|Security|CI|Tests)' ) diff --git a/docs/source/changelog.md b/docs/source/changelog.md index 5e9e8c3f..83bfe633 100644 --- a/docs/source/changelog.md +++ b/docs/source/changelog.md @@ -4,6 +4,14 @@ All notable changes to this project will be documented in this file. Dates are d Generated by [`auto-changelog`](https://github.com/CookPete/auto-changelog). +## [Unreleased](https://github.com/craigerl/aprsd/compare/5.0.1...HEAD) + +### Refactoring + +- refactor: self-registering packet registry in packets/core [`72541bf`](https://github.com/craigerl/aprsd/commit/72541bf) +- refactor: compose Position value object into GPSPacket [`f30f764`](https://github.com/craigerl/aprsd/commit/f30f764) +- Non-breaking: GPS packet types still accept the legacy flat position kwargs (latitude=, longitude=, ...) and flat attribute reads (packet.latitude); the legacy form is deprecated and will be removed in a future release — use position=Position(...) [`f30f764`](https://github.com/craigerl/aprsd/commit/f30f764) + ## [5.0.1](https://github.com/craigerl/aprsd/compare/5.0.0...5.0.1) ### Bug Fixes