From e8149c14f8cc296909203f63af0f801639429b55 Mon Sep 17 00:00:00 2001 From: Igor Panteleyev Date: Thu, 23 Jul 2026 15:23:42 +0300 Subject: [PATCH] Improve history processing --- .gitignore | 3 ++ docs/api/configuration.md | 2 + docs/protocol.md | 35 ++++++++++--- pyproject.toml | 2 +- src/pycomap/configuration.py | 89 +++++++++++++++++++++++++++----- src/pycomap/controller.py | 32 ++++++++++-- tests/unit/builders.py | 45 ++++++++++++++++ tests/unit/test_configuration.py | 38 +++++++++++--- tests/unit/test_controller.py | 74 +++++++++++++++++++++++++- uv.lock | 2 +- 10 files changed, 287 insertions(+), 35 deletions(-) diff --git a/.gitignore b/.gitignore index c64c70f..52bedb4 100644 --- a/.gitignore +++ b/.gitignore @@ -22,3 +22,6 @@ htmlcov/ # MkDocs build output site/ + +# DLLs +dlls diff --git a/docs/api/configuration.md b/docs/api/configuration.md index 95265e1..53b326d 100644 --- a/docs/api/configuration.md +++ b/docs/api/configuration.md @@ -12,6 +12,8 @@ these internally — you only need them directly when working with raw blobs. ::: pycomap.configuration.ValueState +::: pycomap.configuration.HistoryFieldDescription + ## Enums ::: pycomap.configuration.ValueCategory diff --git a/docs/protocol.md b/docs/protocol.md index 3ec8f32..b28ab1b 100644 --- a/docs/protocol.md +++ b/docs/protocol.md @@ -696,12 +696,35 @@ total hours and bits 24-31 = minutes. - **Text records** (`prefix_index == 30`): null-terminated ASCII string describing the configuration change (e.g. `"T=ETH CA1 A CON(24554)=21:25:08"`). -- **Alarm/event records**: a snapshot of the first 57 bytes of `ValuesAll`, captured at the - moment of the event. The byte layout is identical to the live `ValuesAll` blob — each - value's raw bytes sit at its `data_index` byte offset, with the same data type and - decimal-place encoding. Values whose `data_index + data_length > 57` are absent from the - snapshot. On the test hardware this covers 31 values: `Binary Inputs` (data_index=0) through - `Load Power Factor` (data_index=56, 1 byte). Decoded via +- **Alarm/event records**: a fixed-format snapshot of a specific set of values, captured at + the moment of the event. **This is not a truncated `ValuesAll`** — an earlier version of + this doc assumed the byte layout matched `ValuesAll`'s `data_index`, which is disproved by + live evidence (e.g. `Binary Outputs` differs between a live `ValuesAll` read and a + same-instant history record's bytes at that offset; `VBat`/`Generator Frequency`/`Mains + Frequency`/`RPM` all have `ValuesAll` `data_index` values past byte 57 yet are populated + in every history row shown by WebSupervisor). + + The real layout comes from a separate `HistoryDescriptionCollection` section of the + `ConfigurationTable` (`ConfigurationTablePart.TerminalPart`), decompiled from + `ComAp.Controller.dll` + (`ConfigurationTableLoaderCommonExtensions.CommonLoadHistoryFromStream` + + `ConfigurationTableLoaderIL3.LoadHistoryItemDescriptionFromStream`, IL3 non-Format7): + + - Offset 152 in the `ConfigurationTable`: `uint16` item count, then a `uint16` padding + field (read but discarded), then a `uint32` absolute address of the item table. + - Each item is one `uint32` LE: bits 0-10 = `val_index` (0-based index into the *stream + order* of parsed `Values` — not a comm-object number, not `ValuesAll`'s `data_index`), + bits 11-19 = `data_index` (byte offset within the 57-byte snapshot, stored directly per + item — not derived by summing previous items' lengths), bits 20-31 = `name_index` (into + `NamesCategory.CommonNames` — a display name for the item, e.g. `"RPM"`, `"VBat"`). + + On the test hardware this is 30 items, in the same order as WebSupervisor's history table + columns: `RPM, PF, LChr, IL1, IL2, IL3, Mfrq, Vm1, Vm2, Vm3, Vm12, Vm23, Vm31, VBat, AIN1, + AIN2, AIN3, AIN4, BIN, BOUT, Gfrq, Vg1, Vg2, Vg3, Vg12, Vg23, Vg31, Q, Pwr, Mode`. Verified + field-for-field (including bit-for-bit `BIN`/`BOUT` and enum-for-enum `Mode`) against a + live controller's WebSupervisor history view. Parsed into + `ConfigurationTable.history_fields` (`pycomap.configuration.HistoryFieldDescription`, + distinct from `ValueDescription.data_index`) and decoded via `pycomap.configuration.decode_history_snapshot(table, record.data)` or `Controller.decode_history_snapshot(record)`. diff --git a/pyproject.toml b/pyproject.toml index 9cf5cc1..6d924cf 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "pycomap" -version = "2.1.1" +version = "2.2.0" description = "Async Python client for ComAp controllers: LAN discovery and the native ECDH/AES-encrypted control protocol" readme = "README.md" license = "MIT" diff --git a/src/pycomap/configuration.py b/src/pycomap/configuration.py index fb190a2..529b371 100644 --- a/src/pycomap/configuration.py +++ b/src/pycomap/configuration.py @@ -53,6 +53,7 @@ __all__ = [ "ConfigurationTable", + "HistoryFieldDescription", "NamesCategory", "ProtectionState", "SetpointCategory", @@ -79,6 +80,11 @@ _NUM_SETPOINTS_CAT_P_OFFSET = 98 _SETPOINT_RECORD_SIZE = 14 +# History snapshot field layout (IL3, ConfigurationTableLoaderIL3.CreateConfigurationTableLoadInfo +# NumHistFieldsOffset=152, IsNumHistFieldsByte=false). Distinct from ValuesAll's data_index -- +# see HistoryFieldDescription and _parse_history_description. +_NUM_HIST_FIELDS_OFFSET = 152 + # Unified names heap (IL3-specific fixed offsets, see module docstring and # docs/protocol.md section 4). _DESCR_LANG_OFFSET = 169 @@ -261,6 +267,21 @@ def any_alarm(self) -> bool: return bool((self.level1 & active) or (self.level2 & active) or (self.sensor_fail & active)) +@dataclass(slots=True, frozen=True) +class HistoryFieldDescription: + """One entry in the controller's history-snapshot field layout. + + Describes where a single value's raw bytes sit within a ``HistoryRecord.data`` snapshot. + This is a **separate** layout from ``ValuesAll`` -- ``data_index`` here is the byte offset + within the 57-byte history snapshot, unrelated to ``value.data_index`` (which is the + offset within ``ValuesAll``/``ValueStatesAndDataAll``). See + [decode_history_snapshot][pycomap.configuration.decode_history_snapshot]. + """ + + data_index: int + value: ValueDescription + + @dataclass(slots=True, frozen=True) class ConfigurationTable: """Parsed ``ConfigurationTable`` (value and setpoint descriptions -- see module @@ -269,6 +290,7 @@ class ConfigurationTable: values: list[ValueDescription] setpoints: list[SetpointDescription] + history_fields: list[HistoryFieldDescription] def _as_int16(v: int) -> int: @@ -330,6 +352,40 @@ def _parse_group_map( return co_to_group +def _parse_history_description( + data: bytes, values: list[ValueDescription] +) -> list[HistoryFieldDescription]: + """Parse the history-snapshot field layout section of the ``ConfigurationTable``. + + Source: ``ConfigurationTableLoaderCommonExtensions.CommonLoadHistoryFromStream`` + + ``ConfigurationTableLoaderIL3.LoadHistoryItemDescriptionFromStream`` in + ``ComAp.Controller.dll`` (IL3, non-Format7 -- ``NumHistFieldsOffset=152``, + ``IsNumHistFieldsByte=false``, ``AreAllAddresses32Bit=true``). + + Layout at offset 152: ``uint16`` item count, ``uint16`` padding (read but discarded by + the non-byte-count branch), ``uint32`` absolute address of the item table. Each item is + one ``uint32`` LE: bits 0-10 = ``val_index`` (0-based index into ``values``, in the same + stream-load order used to build that list -- *not* a comm-object number or a + ``ValuesAll`` ``data_index``), bits 11-19 = ``data_index`` (byte offset within the + history snapshot payload, stored directly per item), bits 20-31 = ``name_index`` (into + ``NamesCategory.COMMON_NAMES`` -- not resolved here; each ``value.name`` already carries + a name). + """ + num_items = struct.unpack_from(" ConfigurationTable: """Parse the value-description section of a raw ``ConfigurationTable`` blob.""" controller_type = data[_CONTROLLER_TYPE_OFFSET] @@ -407,7 +463,9 @@ def parse_configuration_table(data: bytes) -> ConfigurationTable: values = [dataclasses.replace(v, group=group_map.get(v.number)) for v in values] setpoints = [dataclasses.replace(s, group=group_map.get(s.number)) for s in setpoints] - return ConfigurationTable(values=values, setpoints=setpoints) + history_fields = _parse_history_description(data, values) + + return ConfigurationTable(values=values, setpoints=setpoints, history_fields=history_fields) def _parse_setpoints( @@ -499,25 +557,28 @@ def decode_values_all(table: ConfigurationTable, data: bytes) -> dict[int, RawVa def decode_history_snapshot(table: ConfigurationTable, snapshot: bytes) -> dict[int, RawValue]: """Decode the value snapshot from a ``HistoryRecord.data`` field. - Alarm/event history records carry a snapshot of the first N bytes of the - ``ValuesAll`` blob captured at the moment the event occurred. The layout is - identical to ``ValuesAll`` (each value at its ``data_index`` byte offset) but - truncated to ``len(snapshot)`` — values whose data would extend beyond the - snapshot are silently omitted. - - Returns ``{number: decoded_value}`` for every value that fits, using the same - type/decimal-places decoding as - [decode_values_all][pycomap.configuration.decode_values_all]. ``ONE_TIME`` values - are never included. Returns an empty dict if ``snapshot`` is empty (text records). + Alarm/event history records carry a fixed-format snapshot of a specific set of values + (RPM, voltages, frequencies, battery voltage, binary I/O, mode, ...), captured at the + moment the event occurred. This layout is defined by the controller's own + ``HistoryDescriptionCollection`` (``table.history_fields`` -- a separate section of the + ``ConfigurationTable``, unrelated to ``ValuesAll``'s per-value ``data_index``) rather than + being a truncated copy of ``ValuesAll``. Verified field-for-field against a live + controller's WebSupervisor history view. + + Returns ``{number: decoded_value}`` for every history field that fits within + ``len(snapshot)``, using the same type/decimal-places decoding as + [decode_values_all][pycomap.configuration.decode_values_all]. Returns an empty dict if + ``snapshot`` is empty (text records). """ result: dict[int, RawValue] = {} - for value in table.values: + for field in table.history_fields: + value = field.value if value.category is ValueCategory.ONE_TIME: continue - end = value.data_index + value.data_length + end = field.data_index + value.data_length if end > len(snapshot): continue - raw = snapshot[value.data_index : end] + raw = snapshot[field.data_index : end] result[value.number] = decode_raw_value(value.data_type, raw, value.decimal_places) return result diff --git a/src/pycomap/controller.py b/src/pycomap/controller.py index 8c6ab07..3103621 100644 --- a/src/pycomap/controller.py +++ b/src/pycomap/controller.py @@ -29,6 +29,7 @@ import datetime import logging import re +import struct from collections.abc import Mapping from types import MappingProxyType, TracebackType from typing import Self @@ -526,10 +527,29 @@ async def read_alarms(self) -> list[AlarmRecord]: return parse_alarm_list(self._config_data, data) async def read_history(self, count: int = 10) -> list[HistoryRecord]: - """Read up to ``count`` of the most recent history records, newest first.""" + """Read up to ``count`` of the most recent history records, newest first. + + ``count`` is transparently capped to the controller's own ring-buffer size — + read fresh each call from ``HistoryLength`` (C.O. 24538, number of currently valid + records — grows from 0 up to ``MaxHistoryRecords`` before the buffer first wraps, + then stays there) and ``MaxHistoryRecords`` (C.O. 24564, the buffer's fixed + capacity). Without this, requesting more records than the buffer holds would + silently wrap around and re-return the newest records a second time. + """ if self._config_data is None: raise ComApProtocolError("not connected — call connect() first") + + history_length = struct.unpack( + " list[HistoryRecord]: def decode_history_snapshot(self, record: HistoryRecord) -> dict[int, Value]: """Decode the value snapshot embedded in an alarm ``HistoryRecord``. - Alarm/event records carry a snapshot of the ``ValuesAll`` blob captured at the - moment of the event. Returns ``{number: decoded_value}`` for every value whose - data fits within the snapshot; the set of values is typically the first ~31 entries - from the controller's value table (those with small ``data_index`` values). + Alarm/event records carry a fixed-format snapshot of a specific set of values (RPM, + voltages, frequencies, battery voltage, binary I/O, mode, ...) captured at the moment + of the event, per the controller's ``HistoryDescriptionCollection`` layout (see + [decode_history_snapshot][pycomap.configuration.decode_history_snapshot]). Returns + ``{number: decoded_value}`` for every history field whose data fits within the + snapshot. Returns an empty dict for text records (``record.is_text=True``) or records with no embedded data. diff --git a/tests/unit/builders.py b/tests/unit/builders.py index e1478fc..464b312 100644 --- a/tests/unit/builders.py +++ b/tests/unit/builders.py @@ -45,6 +45,38 @@ def value_record( return struct.pack(" int: + return ((v // 10) << 4) | (v % 10) + + +def history_record( + *, + reason_index: int = 0, + reason_category: int = 0, + prefix_index: int = 0, + level: int = 0, + index: int = 0, + day: int = 1, + month: int = 1, + year: int = 26, + hour: int = 0, + minute: int = 0, + second: int = 0, + payload: bytes = b"", +) -> bytes: + """Build one 69-byte ``HistoryRecord`` wire blob (see ``pycomap.history`` module docstring). + + Defaults to a valid, resolvable wall-clock (RTC) record with a zeroed payload. + """ + word = (reason_index & 0xFFF) | ((reason_category & 0x3) << 13) + flags_byte = (prefix_index & 0x1F) | ((level & 0x7) << 5) + date_time_bytes = bytes( + [_bcd(day), _bcd(month), _bcd(year), _bcd(hour), _bcd(minute), _bcd(second)] + ) + ts = date_time_bytes + bytes([0]) + struct.pack(" bytes: + """``history_items`` is a sequence of ``(val_index, data_index, name_index)`` tuples -- + see ``pycomap.configuration._parse_history_description``. ``val_index`` is a 0-based + index into ``numbers``/``records`` (declaration order, not a comm-object number). + """ header = bytearray(_NAMES_HEAP_END) header[5] = 4 # ConfigFormatTerminal > 3 → names-heap access vector items are uint32 header[6] = controller_type @@ -119,4 +156,12 @@ def build_table( struct.pack_into(" None: def test_decode_history_snapshot_decodes_fitting_values() -> None: - # Two UNSIGNED16 values at data_index 0 and 2; snapshot is exactly 4 bytes. + # Two UNSIGNED16 values; history-snapshot layout puts them at data_index 0 and 2 + # (independent of their ValuesAll data_index, set to something else here to prove that). data = _build_table( category_counts=(2, 0, 0, 0), numbers=[10, 20], records=[ - _value_record(data_type=DataType.UNSIGNED16, data_index=0), - _value_record(data_type=DataType.UNSIGNED16, data_index=2), + _value_record(data_type=DataType.UNSIGNED16, data_index=99), + _value_record(data_type=DataType.UNSIGNED16, data_index=98), ], + history_items=[(0, 0, 0), (1, 2, 0)], ) table = parse_configuration_table(data) snapshot = struct.pack(" None: def test_decode_history_snapshot_omits_values_beyond_snapshot() -> None: - # First value fits in 2-byte snapshot; second (data_index=2) does not. + # First value fits in 2-byte snapshot; second (history data_index=2) does not. data = _build_table( category_counts=(2, 0, 0, 0), numbers=[10, 20], records=[ - _value_record(data_type=DataType.UNSIGNED16, data_index=0), - _value_record(data_type=DataType.UNSIGNED16, data_index=2), + _value_record(data_type=DataType.UNSIGNED16, data_index=99), + _value_record(data_type=DataType.UNSIGNED16, data_index=98), ], + history_items=[(0, 0, 0), (1, 2, 0)], ) table = parse_configuration_table(data) snapshot = struct.pack(" None: category_counts=(1, 0, 0, 0), numbers=[10], records=[_value_record(data_type=DataType.UNSIGNED16, data_index=0)], + history_items=[(0, 0, 0)], ) table = parse_configuration_table(data) assert decode_history_snapshot(table, b"") == {} def test_decode_history_snapshot_skips_one_time_values() -> None: - # ONE_TIME values (category index 3) are never in the snapshot. + # ONE_TIME values (category index 3) are never in the snapshot, even if referenced by + # the history-description table. data = _build_table( category_counts=(0, 0, 0, 1), # one ONE_TIME value numbers=[99], records=[_value_record(data_type=DataType.UNSIGNED16, data_index=0)], + history_items=[(0, 0, 0)], ) table = parse_configuration_table(data) snapshot = struct.pack(" None: assert 99 not in result assert result == {} + + +def test_parse_history_description_resolves_order_and_offset() -> None: + # val_index refers to declaration order (not comm-object number); data_index/name_index + # come straight from the history-description entry, independent of ValuesAll's data_index. + data = _build_table( + category_counts=(2, 0, 0, 0), + numbers=[10, 20], + records=[ + _value_record(data_type=DataType.UNSIGNED16, data_index=99), + _value_record(data_type=DataType.UNSIGNED8, data_index=98), + ], + history_items=[(1, 0, 0), (0, 1, 0)], # deliberately reversed order + ) + table = parse_configuration_table(data) + + assert [f.value.number for f in table.history_fields] == [20, 10] + assert [f.data_index for f in table.history_fields] == [0, 1] diff --git a/tests/unit/test_controller.py b/tests/unit/test_controller.py index 270a167..d07a3e8 100644 --- a/tests/unit/test_controller.py +++ b/tests/unit/test_controller.py @@ -17,7 +17,8 @@ ) from pycomap.controller import Controller, _encode_setpoint_value, _parse_gmt_label from pycomap.exceptions import ComApAuthError, ComApProtocolError -from tests.unit.builders import build_table, setpoint_record, value_record +from pycomap.protocol.objects import CommunicationObject +from tests.unit.builders import build_table, history_record, setpoint_record, value_record # --------------------------------------------------------------------------- # _parse_gmt_label @@ -778,3 +779,74 @@ async def test_elevate_access_is_idempotent(mocker) -> None: await ctrl.elevate_access() ctrl._client.elevate_access.assert_not_called() + + +# --------------------------------------------------------------------------- +# read_history — capping to HistoryLength / MaxHistoryRecords +# --------------------------------------------------------------------------- + + +def _ctrl_for_history(mocker): + from unittest.mock import AsyncMock + + table_data = build_table(category_counts=(0, 0, 0, 0), numbers=[], records=[]) + ctrl = _make_ctrl_with_table(mocker, table_data) + ctrl._config_data = table_data + mock_client = AsyncMock() + ctrl._client = mock_client + return ctrl, mock_client + + +async def test_read_history_caps_to_history_length_when_buffer_not_full(mocker) -> None: + """HistoryLength (3) is below both the requested count (10) and MaxHistoryRecords (350) + -- e.g. a controller whose ring buffer hasn't filled up yet.""" + ctrl, mock_client = _ctrl_for_history(mocker) + mock_client.read_object.side_effect = [ + struct.pack(" None: + """Requested count (400) exceeds both HistoryLength and MaxHistoryRecords (5, i.e. a + fully-wrapped ring buffer) -- must not silently wrap around and re-return records.""" + ctrl, mock_client = _ctrl_for_history(mocker) + mock_client.read_object.side_effect = [ + struct.pack(" None: + """A brand-new controller with no logged events yet -- must not attempt any + YOUNGEST_HISTORY_RECORD/OLDER_HISTORY_RECORD read.""" + ctrl, mock_client = _ctrl_for_history(mocker) + mock_client.read_object.side_effect = [ + struct.pack("