docs(da12): spec for subnet-bits dual display + mask-aware edit modal
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
178
docs/subnet-mask-handling-handoff.md
Normal file
178
docs/subnet-mask-handling-handoff.md
Normal file
@@ -0,0 +1,178 @@
|
||||
# DA-12C Subnet Mask / Subnet-Bits Handling — Service-Tool Implementation Spec
|
||||
|
||||
**Audience:** the DA-12C service-tool (Python) project.
|
||||
**Scope:** host-side only. No DA-12 firmware change is required or authorized.
|
||||
**Status:** verified against DA-12C firmware. This document is self-contained — you do not
|
||||
need access to the firmware to implement against it.
|
||||
|
||||
---
|
||||
|
||||
## 1. What the DA-12C stores
|
||||
|
||||
The DA-12C stores its subnet setting as a single byte, `xpSubnetBits`.
|
||||
|
||||
**This byte is the count of *host* bits — the number of trailing *zero* bits in the subnet
|
||||
mask. It is NOT the CIDR prefix length.** This is the legacy Lantronix XPort encoding the
|
||||
firmware inherited.
|
||||
|
||||
```
|
||||
xpSubnetBits = 32 − CIDR_prefix = number of trailing zero bits in the mask
|
||||
|
||||
255.255.255.0 (/24) -> stored value 8 (8 host/zero bits)
|
||||
255.255.0.0 (/16) -> stored value 16
|
||||
255.0.0.0 (/8) -> stored value 24
|
||||
```
|
||||
|
||||
> ⚠️ **The naming trap that has caused field confusion.** The field is labeled
|
||||
> "Subnet bits," which most people read as the CIDR prefix (24 for a /24). It is the opposite
|
||||
> end — the host-bit count. A user who wants 255.255.255.0 and enters `24` actually gets
|
||||
> **255.0.0.0** (a /8). To get 255.255.255.0 they must enter `8`. Surfacing the dotted mask
|
||||
> alongside the stored number (this spec) is intended to remove that ambiguity.
|
||||
|
||||
When the DA-12C applies the setting, it converts the stored byte back to a dotted mask with:
|
||||
|
||||
```c
|
||||
nm = 0xFFFFFFFF & ~((1UL << xpSubnetBits) - 1); // host-bit count -> dotted mask
|
||||
```
|
||||
|
||||
This round-trips correctly for any contiguous mask, including non-octet-aligned ones
|
||||
(/23, /28, etc.). Nothing about the stored representation changes — this spec only adds a
|
||||
friendlier display and an alternate input format.
|
||||
|
||||
---
|
||||
|
||||
## 2. What to build in the service tool
|
||||
|
||||
### 2.1 Display
|
||||
Show **both** the stored host-bit count **and** the equivalent dotted subnet mask, so the
|
||||
user sees what is stored and what it means. Example:
|
||||
|
||||
```
|
||||
Subnet bits: 8 (mask 255.255.255.0)
|
||||
```
|
||||
|
||||
### 2.2 Edit
|
||||
Accept **either** input format and store the host-bit count unchanged:
|
||||
|
||||
- A bare integer `0–32` → interpreted as the host-bit count (the legacy method, preserved
|
||||
exactly so existing users are unaffected).
|
||||
- A dotted-decimal mask (contains `.`) → converted to the host-bit count and stored.
|
||||
|
||||
Disambiguate by the presence of a `.`: contains a dot → parse as a mask; otherwise → parse
|
||||
as an integer host-bit count.
|
||||
|
||||
The stored value written back to the device is always the host-bit count. The legacy entry
|
||||
method is **not** deprecated.
|
||||
|
||||
---
|
||||
|
||||
## 3. Conversion rules
|
||||
|
||||
**Mask → stored value:**
|
||||
```
|
||||
stored = 32 − prefix_length (prefix_length = count of leading 1-bits)
|
||||
```
|
||||
Validate the mask is **contiguous** (all 1-bits followed by all 0-bits) before converting.
|
||||
Reject masks like `255.0.255.0`.
|
||||
|
||||
**Stored value → mask (for display; mirrors the firmware):**
|
||||
```
|
||||
mask = 0xFFFFFFFF & ~((1 << stored) - 1)
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 4. Edge cases — handle these host-side
|
||||
|
||||
- **Default sentinels.** Stored `0` and `255` are both treated by the firmware as
|
||||
"default → 255.255.255.0." When displaying a stored `0` or `255`, show the mask as
|
||||
`255.255.255.0` and label it the default. Do **not** convert an entered mask of
|
||||
255.255.255.0 to stored `0` — store `8` for it (the normal value). Leave an existing
|
||||
stored `0`/`255` untouched unless the user edits the field.
|
||||
- **Clamp valid stored values to 1–31.** The firmware computes `1UL << stored`, which is
|
||||
undefined behavior when `stored >= 32`, and the device only guards the `0`/`255` sentinels
|
||||
on its primary (NetBurner) path. Reject anything outside 1–31, other than the `0`/`255`
|
||||
sentinels you may read back from a device.
|
||||
- **Reject `0.0.0.0` (/0).** Maps to stored 32 (undefined behavior on the device). Not valid.
|
||||
- **Reject `255.255.255.255` (/32).** Maps to host bits 0, which collides with the "default"
|
||||
sentinel and cannot be represented. Not meaningful for this device.
|
||||
|
||||
---
|
||||
|
||||
## 5. Round-trip reference table
|
||||
|
||||
| Dotted mask | CIDR | Stored `xpSubnetBits` |
|
||||
|---|---|---|
|
||||
| 255.255.255.252 | /30 | 2 |
|
||||
| 255.255.255.240 | /28 | 4 |
|
||||
| 255.255.255.128 | /25 | 7 |
|
||||
| 255.255.255.0 | /24 | 8 |
|
||||
| 255.255.254.0 | /23 | 9 |
|
||||
| 255.255.252.0 | /22 | 10 |
|
||||
| 255.255.0.0 | /16 | 16 |
|
||||
| 255.0.0.0 | /8 | 24 |
|
||||
|
||||
---
|
||||
|
||||
## 6. Reference helpers (Python)
|
||||
|
||||
```python
|
||||
def mask_to_subnet_bits(mask: str) -> int:
|
||||
"""Dotted-decimal mask -> stored host-bit count. Raises ValueError if invalid."""
|
||||
octets = [int(o) for o in mask.split(".")]
|
||||
if len(octets) != 4 or any(o < 0 or o > 255 for o in octets):
|
||||
raise ValueError(f"Invalid mask: {mask}")
|
||||
value = (octets[0] << 24) | (octets[1] << 16) | (octets[2] << 8) | octets[3]
|
||||
# Must be contiguous 1s followed by contiguous 0s
|
||||
inverted = (~value) & 0xFFFFFFFF # host portion as 1s
|
||||
if inverted & (inverted + 1) != 0:
|
||||
raise ValueError(f"Non-contiguous mask: {mask}")
|
||||
host_bits = bin(inverted).count("1") # = trailing zeros of the mask
|
||||
if not (1 <= host_bits <= 31):
|
||||
raise ValueError(f"Mask out of supported range (1-31 host bits): {mask}")
|
||||
return host_bits
|
||||
|
||||
|
||||
def subnet_bits_to_mask(stored: int) -> str:
|
||||
"""Stored host-bit count -> dotted-decimal mask (mirrors firmware)."""
|
||||
if stored in (0, 255): # firmware default sentinels
|
||||
return "255.255.255.0"
|
||||
if not (1 <= stored <= 31):
|
||||
raise ValueError(f"Unsupported stored subnet bits: {stored}")
|
||||
nm = 0xFFFFFFFF & ~((1 << stored) - 1)
|
||||
return f"{(nm >> 24) & 0xFF}.{(nm >> 16) & 0xFF}.{(nm >> 8) & 0xFF}.{nm & 0xFF}"
|
||||
|
||||
|
||||
def parse_subnet_input(text: str) -> int:
|
||||
"""Accept either a dotted mask or a bare host-bit count; return stored value."""
|
||||
text = text.strip()
|
||||
if "." in text:
|
||||
return mask_to_subnet_bits(text)
|
||||
n = int(text)
|
||||
if not (1 <= n <= 31):
|
||||
raise ValueError(f"Host-bit count out of range (1-31): {n}")
|
||||
return n
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 7. Acceptance checks
|
||||
|
||||
- Entering `8` and entering `255.255.255.0` both store `8` and display `8 (mask 255.255.255.0)`.
|
||||
- Reading a device with stored `8` displays both forms; writing it back is unchanged.
|
||||
- Stored `0`/`255` display as `255.255.255.0` (default) and are left untouched unless the user
|
||||
edits the field.
|
||||
- Non-contiguous masks, `0.0.0.0`, `255.255.255.255`, and values outside 1–31 are rejected
|
||||
with a clear message.
|
||||
|
||||
---
|
||||
|
||||
## 8. Provenance (firmware references, informational)
|
||||
|
||||
Not needed to implement, recorded for traceability:
|
||||
|
||||
- `config.h:155` — `xpSubnetBits` is a single `unsigned char`.
|
||||
- `op05.c:689` — the misleading "Subnet bits (0=class A B C)" menu label.
|
||||
- `netburner.c:480–494` — the active DA-12C conversion path (sentinels `0`/`255` → default;
|
||||
`nm = 0xFFFFFFFF & ~((1UL << bits) - 1)`).
|
||||
- `rn131.c:580`, `wroom.c:267` — the same conversion on the legacy Wi-Fi paths.
|
||||
@@ -0,0 +1,200 @@
|
||||
# DA-12 Subnet Bits — Dual Display + Mask-Aware Edit Modal
|
||||
|
||||
**Date:** 2026-06-04
|
||||
**Module:** DA-12 (`cim_suite/modules/da12/`)
|
||||
**Status:** Design approved, ready for implementation plan.
|
||||
**Source of truth for the protocol:** `docs/subnet-mask-handling-handoff.md`
|
||||
(verified against DA-12C firmware; self-contained). This spec is the *UI/UX and
|
||||
code-placement* design that consumes it.
|
||||
|
||||
---
|
||||
|
||||
## 1. Problem
|
||||
|
||||
The DA-12C stores its subnet setting as a single byte, `xpSubnetBits` — the count
|
||||
of **host bits** (trailing *zero* bits in the mask), **not** the CIDR prefix. The
|
||||
Station tab currently surfaces this as a bare number (e.g. `8`) edited inline. The
|
||||
label "Subnet bits" reads to most users as a CIDR prefix, so someone wanting
|
||||
`255.255.255.0` types `24` and silently gets `255.0.0.0` (a /8). This is a known
|
||||
field-confusion trap (handoff doc §1).
|
||||
|
||||
## 2. Goal
|
||||
|
||||
Remove the ambiguity, host-side only, with **no firmware change and no wire-format
|
||||
change**:
|
||||
|
||||
1. **Display** the Subnet 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 full
|
||||
dotted-decimal mask, and always stores the host-bit count the device expects.
|
||||
|
||||
The legacy bare-integer entry method is preserved exactly; existing stored values
|
||||
(including the `0`/`255` default sentinels) are never silently rewritten.
|
||||
|
||||
## 3. Non-goals
|
||||
|
||||
- No change to the DA-12 wire protocol, `encoder.py`, or `controller` write path —
|
||||
the modal writes the host-bit integer through the **existing**
|
||||
`set_station_setting`, so the reboot banner and frame format are untouched.
|
||||
- No generic "custom cell editor" framework in `TableTab` (YAGNI for one field).
|
||||
- DA-07 / IOModbus are out of scope (this is DA-12C-specific firmware behavior).
|
||||
|
||||
## 4. Architecture
|
||||
|
||||
Three isolated pieces. The conversion logic is pure (no Qt, no I/O) and fully unit
|
||||
tested; the dialog is a thin view; the Station-tab change is a small, label-keyed hook.
|
||||
|
||||
### 4.1 Pure conversion module — `cim_suite/modules/da12/protocol/subnet.py`
|
||||
|
||||
Lifted from handoff doc §6, sits beside `codecs.py` (the other firmware-mirroring
|
||||
representation code). No Qt, no I/O.
|
||||
|
||||
```python
|
||||
def mask_to_subnet_bits(mask: str) -> int
|
||||
# Dotted-decimal mask -> stored host-bit count. Validates 4 octets 0-255,
|
||||
# contiguous 1s-then-0s, result in 1-31. Raises ValueError otherwise.
|
||||
|
||||
def subnet_bits_to_mask(stored: int) -> str
|
||||
# Stored host-bit count -> dotted-decimal mask (mirrors the firmware
|
||||
# nm = 0xFFFFFFFF & ~((1 << stored) - 1)). Sentinels 0/255 -> "255.255.255.0".
|
||||
# Raises ValueError for stored outside {0,255} ∪ [1,31].
|
||||
|
||||
def parse_subnet_input(text: str) -> int
|
||||
# Accept either form; disambiguate by presence of ".". Dot -> mask path;
|
||||
# else int host-bit count constrained to 1-31. Returns the stored value.
|
||||
|
||||
def format_subnet_display(raw: str) -> str
|
||||
# Grid display helper. "8" -> "8 (mask 255.255.255.0)";
|
||||
# "0"/"255" -> "0 (default — mask 255.255.255.0)";
|
||||
# anything that doesn't parse to an int in {0,255}∪[1,31] -> the raw text
|
||||
# verbatim (never raises — must not crash the grid on unexpected device data).
|
||||
```
|
||||
|
||||
Validation/rejection rules come verbatim from handoff doc §3–§4: reject
|
||||
non-contiguous masks (e.g. `255.0.255.0`), `0.0.0.0` (/0), `255.255.255.255` (/32),
|
||||
and any host-bit count outside 1–31 (other than the `0`/`255` sentinels that may be
|
||||
*read back* from a device).
|
||||
|
||||
### 4.2 Modal — `cim_suite/modules/da12/ui/subnet_dialog.py`
|
||||
|
||||
`SubnetDialog(QDialog)`, peer of the existing `calibration_dialog.py` /
|
||||
`com_setup_dialog.py`. Built from `theme` tokens only (no hard-coded colors/px, no
|
||||
`setStyleSheet` in module code — per `docs/DESIGN-SYSTEM.md`).
|
||||
|
||||
Layout (two synced fields + live preview):
|
||||
|
||||
```
|
||||
┌─ Edit Subnet Setting ──────────────────────┐
|
||||
│ Host bits (0–31): [ 8 ] │
|
||||
│ Subnet mask: [ 255.255.255.0 ] │
|
||||
│ │
|
||||
│ Will store: 8 (mask 255.255.255.0, /24) │
|
||||
│ [ Cancel ] [ Save ] │
|
||||
└────────────────────────────────────────────┘
|
||||
```
|
||||
|
||||
Behavior:
|
||||
|
||||
- **Two-way sync.** Editing *Host bits* recomputes *Subnet mask* + preview; editing
|
||||
*Subnet mask* recomputes *Host bits* + preview. The field being typed in is never
|
||||
overwritten — only the *other* field and the preview update. Programmatic updates
|
||||
block the widgets' signals during the write (mirrors the table's `_loading` guard)
|
||||
to avoid a feedback loop.
|
||||
- **Live validation.** On invalid input the preview line becomes a red error message
|
||||
(e.g. *"Non-contiguous mask"*, *"Host bits must be 1–31"*) sourced from the
|
||||
`ValueError` text, and **Save is disabled**. Valid input shows
|
||||
`Will store: <n> (mask <dotted>, /<cidr>)`.
|
||||
- **Result.** On Save the dialog exposes the committed stored host-bit count (int).
|
||||
Entering `255.255.255.0` yields `8`, never the `0` sentinel.
|
||||
|
||||
#### Sentinel / default prefill (handoff doc §4 + §7)
|
||||
|
||||
When opened on a stored `0` or `255`: prefill Host bits with the sentinel and Subnet
|
||||
mask with `255.255.255.0`; preview reads *"default (255.255.255.0)"*; **Save stays
|
||||
disabled until the user enters a valid 1–31 host-bit count or a valid mask.** Opening
|
||||
and closing (or Cancel) without editing writes **nothing** — satisfying "leave an
|
||||
existing stored `0`/`255` untouched unless the user edits the field."
|
||||
|
||||
### 4.3 Station-tab hook — `cim_suite/modules/da12/ui/station_tab.py`
|
||||
|
||||
The Subnet Bits row is identified by **normalized label**
|
||||
(`normalize_label("Subnet Bits")`), the same stable key `reboot_settings.py` and the
|
||||
help system use — row indices are explicitly unreliable for the LAN settings.
|
||||
|
||||
Changes in `StationTab`:
|
||||
|
||||
1. **Display.** In `rebuild`, when a row's normalized label is the subnet label,
|
||||
render its Value cell via `format_subnet_display(value)` instead of the raw value.
|
||||
2. **Disable inline edit for that one cell.** After `set_rows`, strip
|
||||
`ItemIsEditable` from the subnet row's Value cell (same mechanism already used for
|
||||
`type_code < 0` read-only rows), so a double-click won't start the inline editor.
|
||||
3. **Open the modal.** Connect `self.table.cellDoubleClicked` to a handler: if the
|
||||
double-clicked row is the subnet row **and** its `type_code >= 0`, construct
|
||||
`SubnetDialog` prefilled from the current raw stored value (recovered from
|
||||
`self._lines[row].text`); on `accept`, call
|
||||
`self._ctrl.set_station_setting(line.row, str(stored), line.type_code)` — the
|
||||
identical write path inline edits use, so the reboot banner fires unchanged.
|
||||
|
||||
No change to `controller.py`, `encoder.py`, `messages.py`, or `reboot_settings.py`.
|
||||
|
||||
### 4.4 Contextual help refresh — `cim_suite/modules/da12/help.py`
|
||||
|
||||
Update the "Subnet Bits" help string to note that the dotted-decimal mask is now an
|
||||
accepted input format and that the stored number is the host-bit (trailing-zero)
|
||||
count, not the CIDR prefix.
|
||||
|
||||
## 5. Data flow
|
||||
|
||||
```
|
||||
device 'E' frame ─▶ controller ─▶ SettingLine "Subnet Bits\t8" ─▶ settingsChanged
|
||||
─▶ StationTab.rebuild ─▶ format_subnet_display("8") ─▶ cell shows "8 (mask 255.255.255.0)"
|
||||
|
||||
user double-clicks subnet cell ─▶ SubnetDialog(prefill raw="8")
|
||||
─▶ user types mask/bits ─▶ parse_subnet_input / subnet_bits_to_mask (live)
|
||||
─▶ Save ─▶ stored:int ─▶ controller.set_station_setting(row, "8", fmt)
|
||||
─▶ encoder (unchanged) ─▶ transport ─▶ reboot banner (already wired)
|
||||
```
|
||||
|
||||
## 6. Testing
|
||||
|
||||
**Unit — `tests/da12/test_subnet.py`** (pure, no Qt):
|
||||
|
||||
- Round-trip table from handoff doc §5 (every `mask ⇄ stored` pair).
|
||||
- Every rejection case from §3–§4: non-contiguous (`255.0.255.0`), `0.0.0.0`,
|
||||
`255.255.255.255`, host bits 0/32/33/-1, malformed masks.
|
||||
- Sentinels: `subnet_bits_to_mask(0)` and `(255)` → `255.255.255.0`;
|
||||
`mask_to_subnet_bits("255.255.255.0")` → `8` (never `0`).
|
||||
- `format_subnet_display`: normal value, sentinel value, and unparseable fallback.
|
||||
|
||||
**UI — `tests/da12/test_station_tab_subnet.py`** (`pytest-qt`, offscreen,
|
||||
fake/stub controller):
|
||||
|
||||
- The subnet row's Value cell renders the dual `8 (mask 255.255.255.0)` form.
|
||||
- The subnet Value cell is not inline-editable; double-clicking it opens the dialog.
|
||||
- Entering `255.255.255.0` and saving calls `set_station_setting` with `"8"`.
|
||||
- Invalid input disables Save.
|
||||
- Opening on a sentinel (`0`) and cancelling calls `set_station_setting` **zero**
|
||||
times.
|
||||
|
||||
Keep `pytest` green and `ruff` clean (part of "done" in this repo).
|
||||
|
||||
## 7. Acceptance checks (from handoff doc §7)
|
||||
|
||||
- Entering `8` and entering `255.255.255.0` both store `8` and display
|
||||
`8 (mask 255.255.255.0)`.
|
||||
- Reading a device with stored `8` displays both forms; writing it back is unchanged.
|
||||
- Stored `0`/`255` display as default `255.255.255.0` and are left untouched unless
|
||||
the user edits the field.
|
||||
- Non-contiguous masks, `0.0.0.0`, `255.255.255.255`, and values outside 1–31 are
|
||||
rejected with a clear message.
|
||||
|
||||
## 8. File-change summary
|
||||
|
||||
| File | Change |
|
||||
|---|---|
|
||||
| `cim_suite/modules/da12/protocol/subnet.py` | **New.** Pure conversion + display helpers. |
|
||||
| `cim_suite/modules/da12/ui/subnet_dialog.py` | **New.** `SubnetDialog` modal. |
|
||||
| `cim_suite/modules/da12/ui/station_tab.py` | Dual display, non-editable subnet cell, double-click → modal. |
|
||||
| `cim_suite/modules/da12/help.py` | Refresh "Subnet Bits" help text. |
|
||||
| `tests/da12/test_subnet.py` | **New.** Pure conversion tests. |
|
||||
| `tests/da12/test_station_tab_subnet.py` | **New.** UI behavior tests. |
|
||||
Reference in New Issue
Block a user