mirror of
https://github.com/MarekWo/mc-webui.git
synced 2026-10-30 23:51:57 +01:00
feat(repeaters): move a region in the hierarchy
Adds "Move to another region" to the per-region menu. Until now the only way to re-parent was to delete the region and add it again under a new parent. `region put <name> <parent>` on an existing name already re-parents it (RegionMap::putRegion), so the command was there; what was missing was doing it safely. Two firmware behaviours make a blind put unsafe, both confirmed live on MarWoj Mobile Observer, and both settled by the one tree read the move now does up front: - The only cycle check is `region->id == parent_id`. Moving a region into one of its own descendants is accepted, and detaches that branch from the wildcard root: it disappears from the `region` listing entirely (printChildRegions recurses down from the root) while still holding table slots. Verified — after `region put zza zzb`, both vanished from the tree while `region get zza` still answered ` zza (zzb) F`. Nothing in the UI could then show them, let alone fix them. Forwarding is not at risk: findMatch scans the flat array and never walks parents. - The put branch sets `region->flags = 0`, so moving a flood-denied region silently re-allows flood, and the reply says `OK - (flood allowed)` without hinting anything changed. Verified: zzb was denied before the move and came back allowed. So the move reads `region` first, refuses self/descendant/wildcard targets, then puts, then re-applies `denyf` when the region was denied. A lost restore is surfaced rather than swallowed — the region has moved but is forwarding traffic it was set to drop. The picker filters the same descendant set client-side, so the guard is not a surprise at the end. Moving a region carries its nested regions with it, which is the point: verified moving zzc (holding zzb) under zza. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
1 parent
328e1a3998
commit
77f6a70b97
8 files changed
+303
-6
No files matched your search
+117
-1
@@ -6816,7 +6816,7 @@ _REGION_TREE_MAX = 160 # firmware's exportTo() buffer; see note above
|
||||
# Actions that only touch the in-RAM map, so the UI must follow them with
|
||||
# `save`. `set_default` and `clear_default` are absent: the firmware persists
|
||||
# those itself.
|
||||
_REGION_DIRTYING = frozenset({'add', 'remove', 'allow_flood', 'deny_flood', 'set_home'})
|
||||
_REGION_DIRTYING = frozenset({'add', 'remove', 'allow_flood', 'deny_flood', 'set_home', 'move'})
|
||||
|
||||
|
||||
def _parse_region_line(line):
|
||||
@@ -6937,6 +6937,91 @@ def repeater_regions_get(public_key):
|
||||
return jsonify({'success': False, 'error': str(e)}), 500
|
||||
|
||||
|
||||
def _region_descendants(entries, name):
|
||||
"""Every region nested under `name`, at any depth."""
|
||||
children = {}
|
||||
for e in entries:
|
||||
children.setdefault(e.get('parent'), []).append(e['name'])
|
||||
found = set()
|
||||
stack = list(children.get(name, ()))
|
||||
while stack:
|
||||
current = stack.pop()
|
||||
if current in found:
|
||||
continue # a pre-existing cycle must not spin here
|
||||
found.add(current)
|
||||
stack.extend(children.get(current, ()))
|
||||
return found
|
||||
|
||||
|
||||
def _repeater_region_move(dm, pk, name, parent):
|
||||
"""Re-parent a region, reading the tree first to do it safely.
|
||||
|
||||
`region put <name> <parent>` on a name that already exists re-parents it
|
||||
(RegionMap::putRegion), which is the only way to move a region without
|
||||
deleting and recreating it. Two firmware behaviours make a blind `put`
|
||||
unsafe, and both are settled by the one tree read this does up front:
|
||||
|
||||
1. **Cycles are not checked.** The firmware refuses only to make a region
|
||||
its own parent (`region->id == parent_id`); moving a region into one of
|
||||
its own descendants is accepted and detaches that whole branch from the
|
||||
root. `printChildRegions()` recurses down from the wildcard, so the
|
||||
branch vanishes from the `region` listing while still occupying table
|
||||
slots and still matching packets (`findMatch()` scans the flat array).
|
||||
It is invisible in the UI and therefore unrecoverable from it, so the
|
||||
descendant set is computed here and the move refused.
|
||||
2. **`put` resets the flags.** The CLI branch sets `region->flags = 0`
|
||||
after every put, so moving a flood-denied region silently re-allows
|
||||
flood. The reply says `OK - (flood allowed)` without mentioning that it
|
||||
changed anything. The pre-move flag is captured from the same read and
|
||||
re-applied with `denyf` afterwards, so a move moves and nothing else.
|
||||
|
||||
Returns the same shape as the other region actions, plus `flood_restored`
|
||||
when the deny had to be re-applied.
|
||||
"""
|
||||
tree = dm.repeater_cmd_wait(pk, 'region', timeout=_SETTINGS_FIELD_TIMEOUT)
|
||||
if not tree.get('success'):
|
||||
return None, tree
|
||||
|
||||
entries = _parse_region_tree(tree.get('reply') or '')
|
||||
by_name = {e['name']: e for e in entries}
|
||||
if name not in by_name:
|
||||
return {'error': f'Unknown region: {name}'}, None
|
||||
if parent and parent not in by_name:
|
||||
return {'error': f'Unknown parent: {parent}'}, None
|
||||
if parent == name:
|
||||
return {'error': 'A region cannot be moved into itself'}, None
|
||||
if parent and parent in _region_descendants(entries, name):
|
||||
return {'error': f'{name} cannot be moved into {parent}, '
|
||||
f'which is already nested under it'}, None
|
||||
|
||||
was_denied = not by_name[name]['flood_allowed']
|
||||
cmd = f'region put {name} {parent}' if parent else f'region put {name}'
|
||||
result = dm.repeater_cmd_wait(pk, cmd, timeout=_SETTINGS_FIELD_TIMEOUT)
|
||||
if not result.get('success'):
|
||||
return None, result
|
||||
|
||||
reply = (result.get('reply') or '').strip()
|
||||
ok = not (reply.lower().startswith('err') or not reply)
|
||||
payload = {'success': True, 'ok': ok, 'action': 'move', 'command': cmd,
|
||||
'reply': reply, 'error': None if ok else (reply or 'Empty reply'),
|
||||
'dirty': ok, 'elapsed_ms': result.get('elapsed_ms')}
|
||||
if not (ok and was_denied):
|
||||
return payload, None
|
||||
|
||||
# Put cleared the deny flag; put it back so the move changed only the
|
||||
# position. A failure here is reported rather than swallowed — the region
|
||||
# has moved but is now forwarding flood traffic it was set to drop.
|
||||
restore = dm.repeater_cmd_wait(pk, f'region denyf {name}',
|
||||
timeout=_SETTINGS_FIELD_TIMEOUT)
|
||||
restore_reply = (restore.get('reply') or '').strip()
|
||||
if restore.get('success') and restore_reply.lower().startswith('ok'):
|
||||
payload['flood_restored'] = True
|
||||
else:
|
||||
payload['flood_restore_failed'] = True
|
||||
payload['flood_restore_error'] = restore_reply or restore.get('error') or 'Request failed'
|
||||
return payload, None
|
||||
|
||||
|
||||
def _region_command(action, name, parent):
|
||||
"""Region action → (CLI command text, None), or (None, error message)."""
|
||||
if action == 'save':
|
||||
@@ -6981,6 +7066,9 @@ def repeater_regions_post(public_key):
|
||||
a mesh round-trip is over a second, and a batch failing halfway would leave
|
||||
the caller unsure what landed. `dirty` in the reply says the change is in
|
||||
RAM only and still needs a `save`.
|
||||
|
||||
`move` is the exception and costs two or three round-trips: it has to read
|
||||
the tree before it can move anything safely — see _repeater_region_move().
|
||||
"""
|
||||
dm = _get_dm()
|
||||
if not dm:
|
||||
@@ -6999,6 +7087,34 @@ def repeater_regions_post(public_key):
|
||||
# would shift the remainder into the next positional argument.
|
||||
if any(c in (name + parent) for c in '\r\n '):
|
||||
return jsonify({'success': False, 'error': 'Invalid region name'}), 400
|
||||
|
||||
if action == 'move':
|
||||
if name == '*':
|
||||
return jsonify({'success': False,
|
||||
'error': 'The wildcard region cannot be moved'}), 400
|
||||
ok_name, err = is_valid_region_name(name)
|
||||
if not ok_name:
|
||||
return jsonify({'success': False, 'error': err}), 400
|
||||
if parent and parent != '*':
|
||||
ok_parent, err = is_valid_region_name(parent)
|
||||
if not ok_parent:
|
||||
return jsonify({'success': False, 'error': f'Invalid parent: {err}'}), 400
|
||||
else:
|
||||
parent = '' # '*' and '' both mean the wildcard root
|
||||
try:
|
||||
payload, failure = _repeater_region_move(dm, pk, name, parent)
|
||||
if failure is not None:
|
||||
return jsonify({'success': False,
|
||||
'error': failure.get('error', 'Region move failed')}), \
|
||||
_repeater_result_status(failure)
|
||||
# A validation refusal comes back without a 'success' key.
|
||||
if 'success' not in payload:
|
||||
return jsonify({'success': False, 'error': payload['error']}), 400
|
||||
return jsonify(payload), 200
|
||||
except Exception as e:
|
||||
logger.error(f"Error moving repeater region: {e}")
|
||||
return jsonify({'success': False, 'error': str(e)}), 500
|
||||
|
||||
cmd, error = _region_command(action, name, parent)
|
||||
if error:
|
||||
return jsonify({'success': False, 'error': error}), 400
|
||||
|
||||
@@ -2147,6 +2147,7 @@ function renderRegionsPane(body) {
|
||||
_regionsData = null;
|
||||
_regionsDirty = false;
|
||||
_regionsBusy = false;
|
||||
_regionMoveName = null;
|
||||
body.innerHTML = `
|
||||
<p class="text-muted small mb-2">${tHtml('rptmgmt.reg.intro')}</p>
|
||||
<div class="d-flex align-items-center gap-2 flex-wrap mb-2">
|
||||
@@ -2189,6 +2190,20 @@ function renderRegionsPane(body) {
|
||||
<div class="form-text">${tHtml('rptmgmt.reg.parent_help')}</div>
|
||||
<div class="small text-danger d-none" id="regAddError"></div>
|
||||
</div>
|
||||
<div class="d-none mb-3" id="regMoveForm">
|
||||
<div class="input-group input-group-sm">
|
||||
<label class="input-group-text" for="regMoveParent" id="regMoveLabel"></label>
|
||||
<select class="form-select" id="regMoveParent"></select>
|
||||
<button type="button" class="btn btn-primary" id="regMoveConfirm">
|
||||
${tHtml('rptmgmt.reg.move_confirm')}
|
||||
</button>
|
||||
<button type="button" class="btn btn-outline-secondary" id="regMoveCancel">
|
||||
${tHtml('common.cancel')}
|
||||
</button>
|
||||
</div>
|
||||
<div class="form-text">${tHtml('rptmgmt.reg.move_help')}</div>
|
||||
<div class="small text-danger d-none" id="regMoveError"></div>
|
||||
</div>
|
||||
<div id="regList"></div>
|
||||
<div class="card mt-3" id="regDefaultCard">
|
||||
<div class="card-body py-2 px-3">
|
||||
@@ -2215,6 +2230,8 @@ function renderRegionsPane(body) {
|
||||
if (e.key === 'Enter') { e.preventDefault(); submitNewRegion(); }
|
||||
});
|
||||
body.querySelector('#regDefaultApply').addEventListener('click', applyDefaultRegion);
|
||||
body.querySelector('#regMoveCancel').addEventListener('click', () => closeRegionMove());
|
||||
body.querySelector('#regMoveConfirm').addEventListener('click', submitRegionMove);
|
||||
loadRegions();
|
||||
}
|
||||
|
||||
@@ -2225,6 +2242,7 @@ function toggleRegionAddForm(show) {
|
||||
form.classList.toggle('d-none', !open);
|
||||
document.getElementById('regAddError').classList.add('d-none');
|
||||
if (open) {
|
||||
closeRegionMove();
|
||||
const input = document.getElementById('regNewName');
|
||||
input.value = '';
|
||||
const parent = document.getElementById('regNewParent');
|
||||
@@ -2238,7 +2256,8 @@ function setRegionsBusy(busy) {
|
||||
const pane = document.getElementById('paneBody');
|
||||
if (!pane) return;
|
||||
pane.querySelectorAll('#regRefreshBtn, #regAddBtn, #regAddConfirm, #regDefaultApply, ' +
|
||||
'#regNewName, #regNewParent, #regDefaultSelect, .reg-action')
|
||||
'#regNewName, #regNewParent, #regDefaultSelect, ' +
|
||||
'#regMoveParent, #regMoveConfirm, .reg-action, .reg-move')
|
||||
.forEach(el => { el.disabled = busy; });
|
||||
const save = document.getElementById('regSaveBtn');
|
||||
if (save) save.disabled = busy || !_regionsDirty;
|
||||
@@ -2289,6 +2308,9 @@ function renderRegionsList() {
|
||||
list.querySelectorAll('.reg-action').forEach(btn => {
|
||||
btn.addEventListener('click', () => runRegionAction(btn.dataset.action, btn.dataset.name));
|
||||
});
|
||||
list.querySelectorAll('.reg-move').forEach(btn => {
|
||||
btn.addEventListener('click', () => openRegionMove(btn.dataset.name));
|
||||
});
|
||||
}
|
||||
|
||||
// The Add form's parent picker: any known region can hold children, and the
|
||||
@@ -2366,6 +2388,10 @@ function regionRowHtml(e) {
|
||||
<i class="bi bi-house me-2"></i>${tHtml('rptmgmt.reg.set_home')}
|
||||
</button></li>`}
|
||||
${e.is_root ? '' : `
|
||||
<li><button type="button" class="dropdown-item reg-move"
|
||||
data-name="${esc(e.name)}">
|
||||
<i class="bi bi-diagram-3 me-2"></i>${tHtml('rptmgmt.reg.move')}
|
||||
</button></li>
|
||||
<li><hr class="dropdown-divider"></li>
|
||||
<li><button type="button" class="dropdown-item text-danger reg-action"
|
||||
data-action="remove" data-name="${esc(e.name)}">
|
||||
@@ -2376,6 +2402,94 @@ function regionRowHtml(e) {
|
||||
</div>`;
|
||||
}
|
||||
|
||||
// ---------------- Moving a region in the hierarchy ----------------
|
||||
// `region put <name> <parent>` re-parents an existing region, which beats
|
||||
// deleting and recreating it. The picker offers every region except the one
|
||||
// being moved and everything already nested under it: the firmware accepts
|
||||
// such a move and the whole branch then disappears from the listing, leaving
|
||||
// nothing in this UI to fix it with. The backend refuses those too.
|
||||
|
||||
let _regionMoveName = null; // region currently being moved, null when closed
|
||||
|
||||
function regionDescendants(name) {
|
||||
const entries = (_regionsData && _regionsData.entries) || [];
|
||||
const children = new Map();
|
||||
entries.forEach(e => {
|
||||
if (!children.has(e.parent)) children.set(e.parent, []);
|
||||
children.get(e.parent).push(e.name);
|
||||
});
|
||||
const found = new Set();
|
||||
const stack = [...(children.get(name) || [])];
|
||||
while (stack.length) {
|
||||
const current = stack.pop();
|
||||
if (found.has(current)) continue; // never spin on an existing cycle
|
||||
found.add(current);
|
||||
stack.push(...(children.get(current) || []));
|
||||
}
|
||||
return found;
|
||||
}
|
||||
|
||||
function openRegionMove(name) {
|
||||
const entries = (_regionsData && _regionsData.entries) || [];
|
||||
const self = entries.find(e => e.name === name);
|
||||
if (!self) return;
|
||||
const blocked = regionDescendants(name);
|
||||
const targets = entries.filter(e => !e.is_root && e.name !== name && !blocked.has(e.name));
|
||||
|
||||
const select = document.getElementById('regMoveParent');
|
||||
select.innerHTML = `<option value="">${esc(t('rptmgmt.reg.parent_root'))}</option>` +
|
||||
targets.map(e => `<option value="${esc(e.name)}">${' '.repeat((e.depth - 1) * 2)}${esc(e.name)}</option>`)
|
||||
.join('');
|
||||
// Start on where it already is, so the picker shows the current position.
|
||||
select.value = (self.parent && self.parent !== '*') ? self.parent : '';
|
||||
|
||||
_regionMoveName = name;
|
||||
document.getElementById('regMoveLabel').textContent = t('rptmgmt.reg.move_label', { name });
|
||||
document.getElementById('regMoveError').classList.add('d-none');
|
||||
document.getElementById('regMoveForm').classList.remove('d-none');
|
||||
toggleRegionAddForm(false);
|
||||
select.focus();
|
||||
}
|
||||
|
||||
function closeRegionMove() {
|
||||
_regionMoveName = null;
|
||||
const form = document.getElementById('regMoveForm');
|
||||
if (form) form.classList.add('d-none');
|
||||
}
|
||||
|
||||
async function submitRegionMove() {
|
||||
if (_regionsBusy || !_regionMoveName) return;
|
||||
const name = _regionMoveName;
|
||||
const select = document.getElementById('regMoveParent');
|
||||
const parent = select.value || '';
|
||||
const entry = ((_regionsData && _regionsData.entries) || []).find(e => e.name === name);
|
||||
const currentParent = (entry && entry.parent && entry.parent !== '*') ? entry.parent : '';
|
||||
if (parent === currentParent) { // already there — don't spend a round-trip
|
||||
closeRegionMove();
|
||||
return;
|
||||
}
|
||||
const errEl = document.getElementById('regMoveError');
|
||||
errEl.classList.add('d-none');
|
||||
const data = await postRegionAction({ action: 'move', name, parent });
|
||||
if (regionActionFailed(data)) {
|
||||
errEl.textContent = data.error || t('rptmgmt.reg.action_failed');
|
||||
errEl.classList.remove('d-none');
|
||||
return;
|
||||
}
|
||||
setRegionsDirty(true);
|
||||
closeRegionMove();
|
||||
// A move clears the firmware's flood flag, so the backend puts a deny back.
|
||||
// If that follow-up is the part that got lost, say so — the region moved but
|
||||
// is now forwarding traffic it was set to drop.
|
||||
if (data.flood_restore_failed) {
|
||||
showNotification(t('rptmgmt.reg.moved_flood_lost', { name }), 'warning');
|
||||
} else {
|
||||
showNotification(parent ? t('rptmgmt.reg.moved', { name, parent })
|
||||
: t('rptmgmt.reg.moved_root', { name }), 'success');
|
||||
}
|
||||
await loadRegions();
|
||||
}
|
||||
|
||||
// A region action can fail two ways: the request never got through (success
|
||||
// false), or the repeater answered and refused (ok false — `Err - not empty`
|
||||
// when a region still has children). Both are failures to the caller, and
|
||||
|
||||
@@ -1014,6 +1014,13 @@
|
||||
"rptmgmt.reg.home": "home",
|
||||
"rptmgmt.reg.intro": "Which flood-scoped packets this repeater passes on. A region set to Flood allowed is repeated; one set to Flood denied is dropped. Changes take effect at once but live in memory only until you save them.",
|
||||
"rptmgmt.reg.menu_title": "Region actions",
|
||||
"rptmgmt.reg.move": "Move to another region",
|
||||
"rptmgmt.reg.move_confirm": "Move",
|
||||
"rptmgmt.reg.move_help": "Choose the region to nest it under, or top level. Its own nested regions move with it, and stay in the same order underneath.",
|
||||
"rptmgmt.reg.move_label": "Move {name} into",
|
||||
"rptmgmt.reg.moved": "Region {name} moved into {parent}",
|
||||
"rptmgmt.reg.moved_flood_lost": "{name} was moved, but its Flood denied setting could not be restored and it is now allowing flood. Set it again.",
|
||||
"rptmgmt.reg.moved_root": "Region {name} moved to the top level",
|
||||
"rptmgmt.reg.name_help": "Letters, digits and - $ # are allowed; no spaces. Max 30 characters.",
|
||||
"rptmgmt.reg.name_placeholder": "Region name, e.g. pl",
|
||||
"rptmgmt.reg.none": "This repeater has no regions configured.",
|
||||
|
||||
@@ -1076,6 +1076,13 @@
|
||||
"rptmgmt.reg.home": "macierzysty",
|
||||
"rptmgmt.reg.intro": "Które pakiety z zakresem regionu ten repeater przekazuje dalej. Region ustawiony na Flood dozwolony jest powtarzany, a ustawiony na Flood zablokowany — odrzucany. Zmiany działają od razu, ale do czasu zapisania istnieją tylko w pamięci.",
|
||||
"rptmgmt.reg.menu_title": "Akcje regionu",
|
||||
"rptmgmt.reg.move": "Przenieś do innego regionu",
|
||||
"rptmgmt.reg.move_confirm": "Przenieś",
|
||||
"rptmgmt.reg.move_help": "Wybierz region, wewnątrz którego ma się znaleźć, albo najwyższy poziom. Zagnieżdżone w nim regiony przenoszą się razem z nim i zachowują swój układ.",
|
||||
"rptmgmt.reg.move_label": "Przenieś {name} do",
|
||||
"rptmgmt.reg.moved": "Region {name} został przeniesiony do {parent}",
|
||||
"rptmgmt.reg.moved_flood_lost": "Region {name} został przeniesiony, ale nie udało się przywrócić ustawienia Flood zablokowany i obecnie zezwala na flood. Ustaw je ponownie.",
|
||||
"rptmgmt.reg.moved_root": "Region {name} został przeniesiony na najwyższy poziom",
|
||||
"rptmgmt.reg.name_help": "Dozwolone są litery, cyfry oraz - $ #; bez spacji. Maks. 30 znaków.",
|
||||
"rptmgmt.reg.name_placeholder": "Nazwa regionu, np. pl",
|
||||
"rptmgmt.reg.none": "Ten repeater nie ma skonfigurowanych regionów.",
|
||||
|
||||
@@ -119,7 +119,7 @@ The `/repeaters` (list) and `/repeaters/manage` (per-repeater tools) panels are
|
||||
- **Login sessions** — `repeater_login()` waits for `LOGIN_SUCCESS` filtered by the contact's `pubkey_prefix` and stores `{is_admin, permissions, logged_in_at}` in the in-memory `_repeater_sessions` dict (cleared on logout/app restart; the UI auto-relogs with the saved password). All remote endpoints fail fast with 401 `need_login` when no session exists — a repeater only answers pubkeys in its ACL, so requesting without a login would just burn a multi-minute timeout. A wrong password is indistinguishable from an unreachable repeater (the firmware never replies to failed logins), so timeout messages always name both causes
|
||||
- **CLI reply correlation** — `repeater_cmd_wait()` sends one text command and blocks for its reply. Replies arrive as `CONTACT_MSG_RECV` with `txt_type=1` (CLI_DATA) and carry no protocol-level correlation, so correlation = the repeater lock (single command in flight) + a single-slot waiter matched against the sender's 12-hex pubkey prefix at the top of `_on_dm_received`. Matched replies are consumed there and never stored as chat DMs; unmatched CLI replies keep the legacy behavior (stored as a DM) so the Console's fire-and-forget `cmd` flow is unchanged. Wait time derives from the device-suggested timeout (clamped 10–45 s)
|
||||
- **Settings batches** — reads run sequential `get <field>` commands per section and parse the firmware's `> value` replies, reporting per-field errors without failing the batch; writes send only dirty fields and classify each reply as `ok` (starts with `ok`/`password now`), `reboot_required` (reply mentions reboot — e.g. `set radio`), or `failed` (anything else, surfaced verbatim). Connection-level failures abort the rest of the batch
|
||||
- **Regions** — the repeater's flood-scope map, edited over text CLI (`region ...`). Two firmware facts drive the design. `put`/`remove`/`allowf`/`denyf` mutate the map **in RAM only** and are lost on reboot until `region save` writes flash, so the UI stages edits behind an explicit Save and warns before navigating away; `region default` is the exception (the firmware calls `saveRegions()` itself) and so never sets the dirty flag. The bare `region` reply is an indented tree — indent = nesting depth, `^` = home region, trailing ` F` = flood allowed — capped at 160 bytes by `exportTo(reply, 160)`, so `_parse_region_tree()` strips the suffixes right-to-left (a region may legitimately be named `F` or end in `^`) and `_region_tree_truncated()` reports a clipped tail rather than hiding it. Names are validated with the shared `is_valid_region_name()` before any command is built, which also blocks CLI injection through a name. A repeater that answers `Err - ...` (a region with children, an unknown name) is enforcing its own rule rather than failing as a transport, so that comes back **200 with `ok: false`** like the sibling `/action` and `/neighbours/remove` endpoints — `dirty` stays false and the caller reads `ok`. The binary `AnonReqType.REGIONS` request is deliberately unused here: it answers with `exportNamesTo()`, a flat list of *allowed* names with no hierarchy, deny flags or home marker
|
||||
- **Regions** — the repeater's flood-scope map, edited over text CLI (`region ...`). Two firmware facts drive the design. `put`/`remove`/`allowf`/`denyf` mutate the map **in RAM only** and are lost on reboot until `region save` writes flash, so the UI stages edits behind an explicit Save and warns before navigating away; `region default` is the exception (the firmware calls `saveRegions()` itself) and so never sets the dirty flag. The bare `region` reply is an indented tree — indent = nesting depth, `^` = home region, trailing ` F` = flood allowed — capped at 160 bytes by `exportTo(reply, 160)`, so `_parse_region_tree()` strips the suffixes right-to-left (a region may legitimately be named `F` or end in `^`) and `_region_tree_truncated()` reports a clipped tail rather than hiding it. Names are validated with the shared `is_valid_region_name()` before any command is built, which also blocks CLI injection through a name. A repeater that answers `Err - ...` (a region with children, an unknown name) is enforcing its own rule rather than failing as a transport, so that comes back **200 with `ok: false`** like the sibling `/action` and `/neighbours/remove` endpoints — `dirty` stays false and the caller reads `ok`. `move` re-parents through `region put <name> <parent>` and is the one action that reads before it writes, because two firmware behaviours make a blind put unsafe: the only cycle check is `region->id == parent_id`, so moving a region into its own descendant is accepted and detaches that branch from the wildcard root — it vanishes from `printChildRegions()` while still holding table slots and still matching in `findMatch()` (which scans the flat array, so forwarding never loops); and the put branch sets `region->flags = 0`, silently re-allowing flood on a denied region. One `region` read supplies both the descendant set and the pre-move flag, and a `denyf` follow-up restores the latter. The binary `AnonReqType.REGIONS` request is deliberately unused here: it answers with `exportNamesTo()`, a flat list of *allowed* names with no hierarchy, deny flags or home marker
|
||||
- **Role gating** — the firmware silently drops text CLI from non-admin logins (which would surface as a timeout), so the CLI/Settings/Actions endpoints reject guest sessions with 403 up front instead
|
||||
- **Actions whitelist** — `advert.zerohop`, `advert` (flood), `clock sync`, `reboot`. `reboot` never replies (the firmware restarts immediately without building one), so a clean send followed by silence is reported as success. Text `erase` is firmware-gated to the USB serial console (`sender_timestamp == 0`), hence no erase in the UI
|
||||
|
||||
@@ -405,7 +405,7 @@ Every mutating endpoint hot-reloads the `ObserverManager`, so broker and setting
|
||||
| POST | `/api/repeaters/<pk>/settings` | Apply dirty fields (`{values}`) as a `set` batch with per-field `ok\|failed\|reboot_required` results; admin only |
|
||||
| POST | `/api/repeaters/<pk>/action` | One-shot action (`{action: zerohop_advert\|flood_advert\|discover_neighbours\|clock_sync\|reboot}`); admin only |
|
||||
| GET | `/api/repeaters/<pk>/regions` | Region map: the parsed `region` tree plus the default flood scope; admin only |
|
||||
| POST | `/api/repeaters/<pk>/regions` | One region action (`{action: add\|remove\|allow_flood\|deny_flood\|set_home\|set_default\|clear_default\|save, name?, parent?}`); admin only |
|
||||
| POST | `/api/repeaters/<pk>/regions` | One region action (`{action: add\|remove\|move\|allow_flood\|deny_flood\|set_home\|set_default\|clear_default\|save, name?, parent?}`); admin only |
|
||||
|
||||
Error mapping is shared across the family: 401 `need_login` (no session), 403 (guest on an admin endpoint), 429 (another repeater operation in progress — single firmware request slot), 503 (device not connected), 504 (timeout: repeater unreachable *or* wrong password — indistinguishable by protocol). Passwords are never returned by any GET (`password_set` boolean only). The panels are served by `GET /repeaters` and `GET /repeaters/manage?pubkey=<64-hex>`.
|
||||
|
||||
|
||||
+3
-1
@@ -707,7 +707,9 @@ The screen lists what the repeater currently knows:
|
||||
- Each region below it shows **Flood allowed** or **Flood denied**. Regions can be nested, and a nested one is indented under its parent
|
||||
- The region marked **home** is the repeater's own place in that hierarchy
|
||||
|
||||
The ⋮ menu on a row offers **Allow flood** / **Deny flood**, **Set as home region**, and **Delete region**. A region that still has regions nested under it cannot be deleted — remove the children first.
|
||||
The ⋮ menu on a row offers **Allow flood** / **Deny flood**, **Set as home region**, **Move to another region**, and **Delete region**. A region that still has regions nested under it cannot be deleted — remove the children first.
|
||||
|
||||
**Move to another region** changes where a region sits in the hierarchy without deleting and re-adding it. Pick the region it should sit inside, or *top level*, and press Move. Anything nested under it moves along with it. The picker leaves out the region itself and everything already nested under it: the repeater would accept such a move, but the whole branch would then drop out of the listing and there would be no way left in this screen to get it back. One detail the firmware hides is handled for you — a move resets the region's flood setting, so a region marked **Flood denied** has that put back straight afterwards; in the rare case that second step is the one that gets lost over the mesh, the app says so rather than leaving it quietly forwarding.
|
||||
|
||||
**Add region** takes a name of up to 30 characters; letters, digits and `-`, `$`, `#` are accepted, spaces are not. The **Inside** picker next to it chooses where the new region goes: leave it on *top level* for an ordinary region, or pick an existing one to nest the new region inside it.
|
||||
|
||||
|
||||
+1
-1
@@ -12,7 +12,7 @@ For deep technical notes, see [architecture.md](architecture.md). For the full g
|
||||
|
||||
### Features
|
||||
|
||||
- **Repeater regions can now be managed from the interface, not just the CLI.** A repeater decides which *flood-scoped* packets it passes on from its own list of regions — a name like `pl` that a sender stamps on a packet, which only repeaters allowing that region will forward. Setting that up meant typing `region` commands into the console and reading back an indented text tree. **My Repeaters → Settings → Regions** now shows that tree as a list: each region with its **Flood allowed** / **Flood denied** state, nested regions indented under their parent, and the **home** region marked. A ⋮ menu on each row allows or denies flood, sets the home region, or deletes it, and **Add region** takes a name plus an **Inside** picker, so a region can be nested inside an existing one or left at the top level. The wildcard row at the top — *Packets without a region scope* — is the rule for ordinary unstamped packets, and can be switched like any other. Below the list, **Default scope** sets the region this repeater stamps on packets it sends itself.
|
||||
- **Repeater regions can now be managed from the interface, not just the CLI.** A repeater decides which *flood-scoped* packets it passes on from its own list of regions — a name like `pl` that a sender stamps on a packet, which only repeaters allowing that region will forward. Setting that up meant typing `region` commands into the console and reading back an indented text tree. **My Repeaters → Settings → Regions** now shows that tree as a list: each region with its **Flood allowed** / **Flood denied** state, nested regions indented under their parent, and the **home** region marked. A ⋮ menu on each row allows or denies flood, sets the home region, **moves the region somewhere else in the hierarchy** (previously only possible by deleting it and adding it again), or deletes it, and **Add region** takes a name plus an **Inside** picker, so a region can be nested inside an existing one or left at the top level. The wildcard row at the top — *Packets without a region scope* — is the rule for ordinary unstamped packets, and can be switched like any other. Below the list, **Default scope** sets the region this repeater stamps on packets it sends itself.
|
||||
|
||||
Changes behave the way the firmware does, and the screen is explicit about it: adding, deleting or switching a region takes effect at once but lives only in the repeater's memory, so an orange banner appears until you press **Save**, and leaving with unsaved changes asks first. Without that save the repeater forgets them at its next reboot. The Default scope card is the exception — the firmware stores that one by itself, and the card says so.
|
||||
|
||||
|
||||
@@ -14,6 +14,7 @@ import pytest
|
||||
from app.routes.api import (
|
||||
_parse_region_tree,
|
||||
_region_command,
|
||||
_region_descendants,
|
||||
_region_tree_truncated,
|
||||
_reply_is_unsupported,
|
||||
)
|
||||
@@ -172,3 +173,53 @@ def test_region_command_rejects(action, name):
|
||||
cmd, err = _region_command(action, name, '')
|
||||
assert cmd is None
|
||||
assert err
|
||||
|
||||
|
||||
# ================================================================
|
||||
# Descendants — the cycle guard behind Settings → Regions → Move
|
||||
# ================================================================
|
||||
#
|
||||
# `region put <name> <parent>` re-parents an existing region, and the firmware
|
||||
# refuses only the direct self-parent case. Moving a region into one of its own
|
||||
# descendants is accepted and detaches that branch from the wildcard root, so it
|
||||
# disappears from the `region` listing entirely (printChildRegions recurses down
|
||||
# from the root) while still occupying table slots. The UI can then no longer
|
||||
# show it, let alone fix it — hence this guard, on both sides.
|
||||
|
||||
# The hierarchy captured from MarWoj Mobile Observer:
|
||||
# * / pl / pl-ma / {pl-krk, pl-kra}, pl / pl-sk
|
||||
_LIVE_TREE = '*^ F\n pl F\n pl-ma F\n pl-krk F\n pl-kra F\n pl-sk F\n'
|
||||
|
||||
|
||||
@pytest.mark.parametrize('name, expected', [
|
||||
('pl', {'pl-ma', 'pl-krk', 'pl-kra', 'pl-sk'}),
|
||||
('pl-ma', {'pl-krk', 'pl-kra'}), # grandchildren count, not just children
|
||||
('pl-krk', set()), # a leaf has none
|
||||
('pl-sk', set()),
|
||||
('*', {'pl', 'pl-ma', 'pl-krk', 'pl-kra', 'pl-sk'}),
|
||||
('nope', set()), # unknown name is simply childless
|
||||
])
|
||||
def test_region_descendants(name, expected):
|
||||
assert _region_descendants(_parse_region_tree(_LIVE_TREE), name) == expected
|
||||
|
||||
|
||||
def test_descendants_terminates_on_a_pre_existing_cycle():
|
||||
"""A map already holding a cycle must not spin the walk forever.
|
||||
|
||||
Reachable in practice: a cycle made through the CLI survives in the table
|
||||
even though the tree stops showing it, so a later read can hand us one.
|
||||
"""
|
||||
cyclic = [
|
||||
{'name': 'a', 'parent': 'b'},
|
||||
{'name': 'b', 'parent': 'a'},
|
||||
{'name': 'c', 'parent': None},
|
||||
]
|
||||
assert _region_descendants(cyclic, 'a') == {'a', 'b'}
|
||||
|
||||
|
||||
def test_moving_into_a_descendant_is_what_the_guard_catches():
|
||||
entries = _parse_region_tree(_LIVE_TREE)
|
||||
# pl-krk sits under pl-ma, so pl-ma must not be movable into it.
|
||||
assert 'pl-krk' in _region_descendants(entries, 'pl-ma')
|
||||
# Sideways between branches is fine.
|
||||
assert 'pl-sk' not in _region_descendants(entries, 'pl-ma')
|
||||
Reference in new issue
Block a user