andy/cimtechniques-service-suite
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

docs(da07): spec for subnet bits/mask dual display and entry (firmware-verified 1-8 limit)

Browse Source
This commit is contained in:
andy
2026-06-12 11:30:49 -04:00
parent d83a61e65f
commit 8ad923657c
1 changed files with 236 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

236
docs/superpowers/specs/2026-06-12-da07-subnet-mask-display-and-input-design.md Normal file
View File

@@ -0,0 +1,236 @@
# DA-07 Subnet Mask Bits — Dual Display + Mask-Aware Edit Modal
**Date:** 2026-06-12
**Module:** DA-07 (`cim_suite/modules/da07/`)
**Status:** Design approved, ready for implementation plan.
**Source of truth for the protocol:** `docs/DA-07 SERVICE-TOOL-ICD.md` §6 (setting
index 9) plus a direct read of the DA-07 firmware source
(`C:\CIMTechniques\Code Review\DA07\netburner.c:464-481`, `service.c:539-541` /
`797-800`, `config.c:135`) performed 2026-06-12 for this design. This spec is the
*UI/UX and code-placement* design that consumes those findings.
---
## 1. Problem
The DA-07 stores its subnet setting as a single byte, `gStation.xpSubnetBits` —
the count of **host bits** (trailing *zero* bits in the mask), **not** the CIDR
prefix. Same legacy Lantronix XPort encoding as the DA-12C, same field-confusion
trap: a user wanting `255.255.255.0` who types `24` gets a /8, silently. The
Station tab currently surfaces the value as a bare number edited inline.
The DA-12 module already solved this (spec
`2026-06-04-da12-subnet-mask-display-and-input-design.md`): dual display
`8 (mask 255.255.255.0)` plus a modal accepting either entry form. This spec
replicates that UX for the DA-07 — **but the DA-07's firmware rules differ in
three hardware-relevant ways**, so the DA-12 code cannot be copied verbatim.
## 2. Firmware findings (verified from DA-07 source, 2026-06-12)
1. **Same encoding, confirmed.** `netburner.c:470-473` comments outright: *"The
number of subnet bits is the number of zeros on the right side of the subnet
mask."* The write path (`service.c:797-800`) stores any byte unclamped and sets
`updateXport` so the NetBurner SBL2e Ethernet module is reconfigured.
2. **Only `0` is a default sentinel.** `netburner.c:464-467` special-cases only
`xpSubnetBits == 0` → `255.255.255.0`. The factory default is `0`
(`config.c:135`). Unlike the DA-12C, **`255` is *not* a sentinel** — it falls
into the broken path below.
3. **The conversion is buggy outside 1–8** (recorded as **BUG-ICD-13**, see §7):
- *Byte-swap precedence bug* (`netburner.c:475-476`):
`netmaskIp & 0x0000FF00 << 8` parses as `netmaskIp & (0x0000FF00 << 8)`, so
the two middle octets are never swapped during the endian reversal. Stored
**9–15** (/23…/17) go to the Ethernet module with their middle octets
exchanged — e.g. stored 9, meant as `255.255.254.0`, is sent as
`255.254.255.0`, a non-contiguous (illegal) mask.
- *16-bit shift overflow* (`netburner.c:474`): `(1 << bits)` is a 16-bit `int`
on the Xmega (the DA-12C path uses `1UL`). For stored **16–255** the shift
collapses to 0 under avr-gcc's shift loop and the firmware sends **`0.0.0.0`**.
This kills the two most common non-/24 masks — `255.255.0.0` (/16, stored 16)
and `255.0.0.0` (/8, stored 24) — and also `255`. (Strictly this is undefined
behavior; "collapses to 0" is what the shipped compiler/MCU does.)
**Net: the DA-07 firmware applies only stored `0` (default) and `1–8`
(masks /24…/31) correctly.** When the station is in DHCP mode the mask is
ignored, which is likely why this never surfaced in the field.
## 3. Goal
Host-side only, **no firmware change and no wire-format change**:
1. **Display** the Subnet Mask Bits row as both the stored host-bit count *and*
the equivalent dotted mask: `8 (mask 255.255.255.0)`.
2. **Edit** through a modal that accepts **either** a host-bit count **or** a
dotted-decimal mask, and always stores the host-bit count.
3. **Range policy (decided): reject outside 1–8.** The tool must never help a
user store a value the DA-07 firmware provably mis-applies. Values 9–255 read
back *from* a device still display, flagged as not applied correctly by this
firmware — never crash, never silently rewrite.
The legacy bare-integer entry method is preserved for 1–8; an existing stored
`0` (default) is never rewritten unless the user edits the field.
## 4. Non-goals
- No change to `encoder.py`, `decoder.py`, or the controller write path — the
modal writes through the existing `set_station_setting(row, value)` (decimal
ASCII per ICD §3.1 / BUG-ICD-00; optimistic local apply per the no-echo rule).
- No extraction of a shared subnet helper into `core` — the DA-12 and DA-07
rules genuinely differ (range, sentinels, flagging); rule-of-three says wait
for a third device. Approach decided 2026-06-12.
- No attempt to warn-but-allow 9–31 (considered and rejected with the user).
- IOModbus untouched. The settings repository needs no change — the stored value
remains the same raw decimal string it is today.
## 5. Architecture
Three isolated pieces, mirroring the DA-12 pattern as it exists *today*
(SettingsList custom rows — the DA-12 spec's `cellDoubleClicked` wiring predates
the kit refit; do not copy it).
### 5.1 Pure conversion module — `cim_suite/modules/da07/protocol/subnet.py`
**New.** Sits beside `codecs.py`. No Qt, no I/O. The module docstring records the
firmware evidence from §2 so the 1–8 limit is traceable.
```python
def mask_to_subnet_bits(mask: str) -> int
# Dotted-decimal mask -> stored host-bit count. Validates 4 octets 0-255 and
# contiguity, then accepts ONLY host bits 1-8 (masks /24../31). Wider masks
# raise ValueError with a message that says why: "the DA-07 firmware only
# applies masks 255.255.255.0-.254 correctly".
def subnet_bits_to_mask(stored: int) -> str
# 0 -> "255.255.255.0" (the DA-07's ONLY default sentinel — 255 is NOT one);
# 1-8 -> dotted mask. Anything else raises ValueError.
def parse_subnet_input(text: str) -> int
# Either form; disambiguate by presence of ".". Dot -> mask path; else a
# bare integer 1-8. Returns the stored value.
def format_subnet_display(raw: str) -> str
# Display helper; never raises. "0" -> "0 (default — mask 255.255.255.0)";
# "1".."8" -> "8 (mask 255.255.255.0)"; "9".."255" -> flagged, e.g.
# "16 (⚠ not applied correctly by DA-07 firmware)" — never shows a mask the
# hardware doesn't actually use; anything unparseable -> raw text verbatim.
```
Rejection rules: non-contiguous masks (`255.0.255.0`), `0.0.0.0`,
`255.255.255.255`, any mask wider than /24 (host bits ≥ 9, e.g. `255.255.0.0`),
host-bit counts outside 1–8. Entering `255.255.255.0` yields `8`, never `0`.
### 5.2 Modal — `cim_suite/modules/da07/ui/subnet_dialog.py`
**New**, adapted from `da12/ui/subnet_dialog.py` (same `ChromeDialog` +
two-synced-fields + live-preview design; `textEdited`-only sync so programmatic
`setText` never recurses; Save gated on validity; `stored_value()` result).
Differences:
- Field label and prompt say **"Host bits (1–8)"** / *"Enter host bits (1–8) or
a subnet mask"*; window title `DA-07 / Edit Subnet Setting`.
- Error text for out-of-range input explains the firmware limitation, not just
the bound — e.g. *"DA-07 firmware only applies masks 255.255.255.0–.254
(host bits 1–8) correctly"*.
- **Prefill on stored `0`:** show `0` + mask `255.255.255.0`, preview *"Default
(255.255.255.0) — enter a value to change"*, Save disabled until a valid edit.
Cancel writes nothing.
- **Prefill on stored 9–255:** show the raw number with the firmware-limitation
message as a (non-red) info state plus Save disabled, so the user sees why the
current value is flagged and can only save a corrected one.
On Save: `controller.set_station_setting(line.row, str(stored))` — decimal
ASCII, identical write path inline edits use; the controller's optimistic local
apply keeps the row consistent until the next refresh.
### 5.3 Wiring — meta, station tab, help
- `da07/ui/station_settings_meta.py`: "Subnet Mask Bits" becomes
`kind="custom"`, hint *"click to edit"*.
- `da07/ui/station_tab.py` (mirrors `da12/ui/station_tab.py`):
- `_add_setting_row`: a `kind == "custom"` branch calling
`settings_list.add_custom(...)`. The wire read-only flag still wins (a
read-only subnet row renders RO, no dialog).
- Connect `settings_list.settingActivated` → open `SubnetDialog` prefilled
with the row's raw stored value; on accept, write as above.
- Value pushes for the subnet row go through `format_subnet_display` (both in
the `rebuild` value loop and initial fill). Identify the row by
`setting_match_key("Subnet Mask Bits")`, the same label-keyed mechanism the
help lookup uses — wire row indices stay out of the UI logic.
- `export_sheet` keeps exporting the **raw** stored value (consistent with
toggles exporting raw wire values).
- `da07/help.py`: rewrite the "Subnet Mask Bits" entry — both entry forms, the
host-bits (not CIDR) meaning, and the 1–8 firmware limit.
## 6. Data flow
```
station ~B frame (idx 9, type 0) ─▶ decoder (str(byte), e.g. "8") ─▶ StationSetting
─▶ settingsChanged ─▶ StationTab.rebuild ─▶ format_subnet_display("8")
─▶ row shows "8 (mask 255.255.255.0)"
user clicks subnet value ─▶ settingActivated ─▶ SubnetDialog(raw="8")
─▶ user types mask or bits ─▶ live sync + validation (subnet.py)
─▶ Save ─▶ set_station_setting(9, "8") ─▶ ~B098… wire write (decimal ASCII)
─▶ optimistic local apply ─▶ settingsChanged ─▶ display reformats
```
## 7. ICD documentation
Add **BUG-ICD-13** to `docs/DA-07 SERVICE-TOOL-ICD.md` §13 describing the two
`netburner.c` conversion bugs (precedence-bug octet scramble for stored 9–15;
16-bit shift collapse to `0.0.0.0` for stored 16–255, undefined behavior caveat)
with `file:line` evidence, plus a pointer from the §6 settings-table row 9. This
keeps the ICD the self-contained firmware record it claims to be.
## 8. Testing
**Unit — `tests/da07/test_subnet.py`** (pure, no Qt):
- Round-trips for stored 1–8 ⇄ masks `255.255.255.254`…`255.255.255.0`.
- Sentinel: `subnet_bits_to_mask(0)` → `255.255.255.0`;
`mask_to_subnet_bits("255.255.255.0")` → `8` (never `0`); `255` is **rejected**
by `subnet_bits_to_mask` (not a sentinel on DA-07).
- Rejections: host bits 0/9/16/24/32, masks `255.255.0.0` and `255.0.0.0`
(valid on DA-12, rejected here), non-contiguous, `0.0.0.0`,
`255.255.255.255`, malformed text.
- `format_subnet_display`: default `0`, normal 1–8, flagged 9/16/255 (no mask
shown), unparseable fallback.
**UI — `tests/da07/test_station_tab_subnet.py` + `tests/da07/test_subnet_dialog.py`**
(`pytest-qt`, offscreen, controller against the simulator or a stub):
- Subnet row renders the dual form; row is a custom row (no inline editor);
activating it opens the dialog.
- Entering `255.255.255.0` and saving writes `set_station_setting` with `"8"`
(and the outbound frame is the decimal `~B` row-9 command).
- Entering `255.255.0.0` blocks Save with the firmware-limitation message.
- Field sync both directions; prefill states for stored `0` and stored `16`;
Cancel on the default writes nothing.
- A wire read-only subnet row renders RO and never opens the dialog.
Keep `pytest` green and `ruff` clean (part of "done" in this repo).
## 9. Acceptance checks
- Entering `8` and entering `255.255.255.0` both store `8` and display
`8 (mask 255.255.255.0)`.
- Entering `16`, `24`, `255.255.0.0`, or `255.0.0.0` is rejected with a message
naming the DA-07 firmware limitation.
- Stored `0` displays as the default and is left untouched unless edited.
- A device reporting stored `16` displays the value flagged (no mask shown);
opening the dialog explains why and only allows saving a valid 1–8 value.
- The wire write remains the existing decimal-ASCII `~B` command; nothing else
about the frame changes.
## 10. File-change summary
| File | Change |
|---|---|
| `cim_suite/modules/da07/protocol/subnet.py` | **New.** Pure conversion + display helpers (DA-07 rules: 1–8, sentinel `0` only, flagging). |
| `cim_suite/modules/da07/ui/subnet_dialog.py` | **New.** `SubnetDialog` modal (1–8 range, firmware-limitation messaging). |
| `cim_suite/modules/da07/ui/station_settings_meta.py` | "Subnet Mask Bits" → `kind="custom"`. |
| `cim_suite/modules/da07/ui/station_tab.py` | Custom-row branch, `settingActivated` → dialog, display via `format_subnet_display`. |
| `cim_suite/modules/da07/help.py` | Rewrite "Subnet Mask Bits" help text. |
| `docs/DA-07 SERVICE-TOOL-ICD.md` | **BUG-ICD-13** (netburner.c conversion bugs) + §6 row-9 pointer. |
| `tests/da07/test_subnet.py` | **New.** Pure conversion tests. |
| `tests/da07/test_subnet_dialog.py` | **New.** Dialog behavior tests. |
| `tests/da07/test_station_tab_subnet.py` | **New.** Station-tab wiring tests. |