You can not select more than 25 topics
Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
4281 lines
202 KiB
4281 lines
202 KiB
# Codex Notes
|
|
|
|
## Current Analysis: Options Static TG Validation
|
|
|
|
### Findings
|
|
|
|
- `bridge_master.py` attempts to validate `TS1_STATIC` and `TS2_STATIC`
|
|
options before parsing them, but the current validation is ineffective.
|
|
- The whitespace removal calls use `re.sub(...)` without assigning the result,
|
|
so whitespace remains in the option value.
|
|
- The pattern `"![\d\,]"` matches a literal exclamation mark followed by a
|
|
digit or comma. It does not mean "any character except digit or comma".
|
|
- Invalid values such as `TS1=91,A92` can reach `int(tg)` and raise
|
|
`ValueError`.
|
|
- That exception is caught by the broad `except Exception` around the whole
|
|
options block, so the options update can abort partway through.
|
|
|
|
### Assumptions
|
|
|
|
- Static TG option strings are intended to contain only decimal TG numbers
|
|
separated by commas.
|
|
- Simple whitespace in static TG option strings is acceptable and should be
|
|
normalized away, even though it is not strictly correct option syntax.
|
|
- Malformed static TG tokens should be rejected cleanly without creating new
|
|
bridge state for that token.
|
|
- Valid fields in the same options string should still be applied when another
|
|
field is invalid, because there is no good feedback mechanism for rejected
|
|
options.
|
|
- TS1 and TS2 static TG options should be treated independently.
|
|
- The existing broad exception handler is not intended as the normal validation
|
|
path for malformed user options.
|
|
|
|
### Unresolved Questions
|
|
|
|
- Should an invalid new options value leave the previous `CONFIG['SYSTEMS'][...]
|
|
['TS1_STATIC']` / `['TS2_STATIC']` value unchanged?
|
|
- Resolved: in a mixed list like `91,A92`, valid token `91` should still be
|
|
applied while invalid token `A92` is skipped.
|
|
|
|
### Protocol-Sensitive Areas
|
|
|
|
- Static TG configuration affects bridge membership and therefore routing
|
|
decisions for HBP packets.
|
|
- TS1 and TS2 static TG handling must remain symmetrical unless there is an
|
|
intentional FreeDMR-specific reason for divergence.
|
|
- Static TG validation must not change packet bytes. It only controls whether
|
|
bridge state is created or reset.
|
|
- Control/local targets such as `6`, `7`, `8`, `9`, `9990`, and `9991..9999`
|
|
should not accidentally become static routes.
|
|
|
|
### Inferred Invariants
|
|
|
|
- Static TG option parsing should normalize transport/config text before
|
|
mutating bridge state.
|
|
- A malformed static TG option should not prevent other valid option fields from
|
|
applying.
|
|
- Static TGs must pass the shared static TG validator before either
|
|
`make_static_tg()` or `reset_static_tg()` is called.
|
|
- Config and runtime option paths should use the same validation rules for TS1
|
|
and TS2.
|
|
- TS1 and TS2 static option lists should be parsed independently so a malformed
|
|
TS1 token does not block valid TS2 changes, and vice versa.
|
|
|
|
### Resolution
|
|
|
|
- Confirmed intended behavior: static TG options are parsed at token level.
|
|
- Simple whitespace is normalized away.
|
|
- Invalid, prohibited or out-of-range tokens are skipped.
|
|
- Valid tokens in the same TS1 or TS2 list still apply.
|
|
- TS1 and TS2 are independent; invalid tokens on one slot do not block valid
|
|
tokens on the other slot.
|
|
|
|
## Current Analysis: Invalid Default Reflector Persisted After Rejection
|
|
|
|
### Findings
|
|
|
|
- `options_config()` rejects invalid non-zero `DEFAULT_REFLECTOR` / `DIAL`
|
|
values for bridge creation when `valid_dial_a_tg_reflector()` returns false.
|
|
- After logging that the default dial-a-TG is prohibited, the function still
|
|
writes the raw option value into
|
|
`CONFIG['SYSTEMS'][_system]['DEFAULT_REFLECTOR']`.
|
|
- This means an invalid value such as `DIAL=1000000` can be "ignored" for bridge
|
|
state but retained in system configuration.
|
|
- Other code, such as disconnected voice announcement logic, reads
|
|
`CONFIG['SYSTEMS'][system]['DEFAULT_REFLECTOR']` directly and may treat any
|
|
positive value as linked/default reflector state.
|
|
|
|
### Assumptions
|
|
|
|
- If a default reflector option is rejected, no default reflector should remain
|
|
set for that master during the current session.
|
|
- Rejected options should not leave `CONFIG` saying a reflector is configured
|
|
when no corresponding reflector bridge was created.
|
|
- If an invalid default reflector is supplied while an old valid default
|
|
reflector is active, the old default should be disabled rather than preserved.
|
|
- The runtime config should reflect the effective routing state, not merely the
|
|
last raw option value seen.
|
|
|
|
### Unresolved Questions
|
|
|
|
- Should this behavior be visible to clients via an error/status path, or is the
|
|
existing log message sufficient?
|
|
- Should rejected `DEFAULT_REFLECTOR` values preserve the old timer/static TG
|
|
changes from the same options string, or should the entire options update be
|
|
rejected atomically?
|
|
|
|
### Protocol-Sensitive Areas
|
|
|
|
- Default reflector state is dial-a-TG reflector state on TS2 and affects where
|
|
a repeater appears linked after startup or options reload.
|
|
- Voice announcements use TG9 TS2 prompts and may report misleading reflector
|
|
state if configuration and bridge state diverge.
|
|
- A rejected default reflector must not create or retune reflector bridge state.
|
|
- The FreeDMR dial-a-TG policy cap remains `999999`; higher values are not valid
|
|
link targets.
|
|
|
|
### Inferred Invariants
|
|
|
|
- A rejected default reflector must be stored as effective
|
|
`DEFAULT_REFLECTOR = 0`.
|
|
- `CONFIG['SYSTEMS'][system]['DEFAULT_REFLECTOR']` should match the active or
|
|
intended default reflector policy state used by bridge creation.
|
|
- Default reflector validation should be applied before both bridge mutation and
|
|
config mutation.
|
|
- RF dial-a-TG and configured default reflector paths should use the same
|
|
policy bounds.
|
|
|
|
### Resolution
|
|
|
|
- Confirmed intended behavior: an invalid default reflector option means no
|
|
default reflector should be set for the current session.
|
|
- Production handling should reset TS2 TG9 reflector state for the affected
|
|
master and persist effective `DEFAULT_REFLECTOR = 0`.
|
|
- Tests should assert both absence of the invalid reflector bridge and
|
|
deactivation of any previously active default reflector.
|
|
|
|
## Current Analysis: Invalid Startup Default Reflector Remains In Config
|
|
|
|
### Findings
|
|
|
|
- `make_default_reflectors()` uses `valid_dial_a_tg_reflector()` before creating
|
|
a startup default reflector bridge.
|
|
- When `CONFIG['SYSTEMS'][system]['DEFAULT_REFLECTOR']` is invalid, prohibited,
|
|
or above the FreeDMR dial-a-TG policy cap, no bridge is created.
|
|
- Invalid startup default reflectors should be logged because otherwise the
|
|
config-file value is silently ignored.
|
|
- The invalid configured value remains in
|
|
`CONFIG['SYSTEMS'][system]['DEFAULT_REFLECTOR']`.
|
|
- Other code paths, including disconnected voice announcement logic, read
|
|
`DEFAULT_REFLECTOR` directly and treat positive values as configured/default
|
|
reflector state.
|
|
|
|
### Assumptions
|
|
|
|
- Startup configuration should follow the same effective-state rule as options
|
|
reload: invalid default reflector means no default reflector for the current
|
|
session.
|
|
- If startup rejects a default reflector, the effective runtime
|
|
`DEFAULT_REFLECTOR` should become `0`.
|
|
- The server should not announce or expose an invalid positive default reflector
|
|
value after rejecting it for bridge creation.
|
|
|
|
### Unresolved Questions
|
|
|
|
- Should invalid startup `DEFAULT_REFLECTOR` values be rewritten only in runtime
|
|
`CONFIG`, or should any persisted/generated config output also normalize them?
|
|
- Should startup log invalid default reflectors at debug level, warning level, or
|
|
leave existing logging style unchanged?
|
|
|
|
### Protocol-Sensitive Areas
|
|
|
|
- Default reflectors connect TS2 TG9 reflector state at startup.
|
|
- System-wide defaults are intended for sparing use and ideally should not be
|
|
used; they remain available for scenarios that require them.
|
|
- Client-requested settings should be treated as preferential over system-wide
|
|
defaults.
|
|
- A positive `DEFAULT_REFLECTOR` value can affect voice announcements and API or
|
|
reporting output even when bridge state was not created.
|
|
- Invalid startup defaults must not create, activate, or retune reflector bridge
|
|
state.
|
|
|
|
### Resolution
|
|
|
|
- Invalid positive startup default reflectors are logged at warning level and
|
|
still do not create reflector bridge state.
|
|
- Confirmed intended behavior: invalid startup default reflectors normalize the
|
|
in-memory effective `DEFAULT_REFLECTOR` to `0` for the current session. This
|
|
does not write back to the config file.
|
|
|
|
### Inferred Invariants
|
|
|
|
- Runtime `CONFIG['SYSTEMS'][system]['DEFAULT_REFLECTOR']` should describe the
|
|
effective default reflector state for the session.
|
|
- Startup and live options should apply the same dial-a-TG default reflector
|
|
policy.
|
|
- Rejecting an invalid startup default reflector should leave no TS2 TG9 default
|
|
reflector active for that system.
|
|
|
|
## Current Analysis: Options Numeric Fields Converted Before Validation
|
|
|
|
### Findings
|
|
|
|
- In `options_config()`, some numeric option fields are converted with `int()`
|
|
before the later validation block that checks `.isdigit()`.
|
|
- `OVERRIDE_IDENT_TG` is converted and assigned before the code reaches the
|
|
validation that logs "`OVERRIDE_IDENT_TG is not an integer`".
|
|
- `VOICE` and `SINGLE` are also converted before any field-specific numeric
|
|
validation.
|
|
- A malformed option such as `IDENTTG=A` or `VOICE=A` can raise `ValueError`
|
|
inside the broad per-system `except Exception` block.
|
|
- That broad exception can abort processing of otherwise valid fields in the
|
|
same options string, which conflicts with the confirmed tolerant-options
|
|
approach used for static TG tokens.
|
|
|
|
### Assumptions
|
|
|
|
- Malformed optional numeric fields should not abort processing of otherwise
|
|
valid fields in the same options string.
|
|
- Invalid numeric option fields should be ignored individually and logged.
|
|
- Valid fields such as `DIAL`, `TIMER`, `TS1`, or `TS2` should still apply when
|
|
an unrelated optional numeric field is malformed.
|
|
- The broad exception handler should remain a last-resort guard, not the normal
|
|
validation path.
|
|
|
|
### Unresolved Questions
|
|
|
|
- Should invalid `VOICE`, `SINGLE`, and `OVERRIDE_IDENT_TG` each preserve their
|
|
previous effective value?
|
|
- Should invalid values for these fields be logged at debug level, matching
|
|
existing option validation, or warning level?
|
|
- Should boolean-like fields accept only `0` and `1`, or continue accepting any
|
|
integer where Python truthiness determines the effective boolean?
|
|
|
|
### Protocol-Sensitive Areas
|
|
|
|
- `OVERRIDE_IDENT_TG` controls where voice ident packets are sent.
|
|
- Voice ident traffic uses generated DMR packets and should not be redirected to
|
|
an invalid TG because of malformed options.
|
|
- Options parsing should not partially abort before dial-a-TG/default/static
|
|
routing changes are considered.
|
|
|
|
### Inferred Invariants
|
|
|
|
- Options fields should be normalized and validated before mutating runtime
|
|
config.
|
|
- Invalid independent option fields should not block valid independent option
|
|
fields.
|
|
- Numeric option parsing should avoid `ValueError` as normal control flow.
|
|
|
|
### Resolution
|
|
|
|
- Client session options now validate numeric fields before mutating runtime
|
|
config.
|
|
- Invalid `VOICE`, `SINGLE`, and `OVERRIDE_IDENT_TG` values are ignored
|
|
independently and do not block valid fields in the same options string.
|
|
- `VOICE` and `SINGLE` accept only `0` and `1`.
|
|
- Empty `DIAL` / `DEFAULT_REFLECTOR` is parsed as `0`, meaning no default
|
|
reflector.
|
|
|
|
## Current Analysis: Voice Ident Override TG Range Check
|
|
|
|
### Findings
|
|
|
|
- `ident()` checks `OVERRIDE_IDENT_TG` before sending generated voice ident
|
|
packets.
|
|
- The upper-bound expression is malformed:
|
|
`int(CONFIG['SYSTEMS'][system]['OVERRIDE_IDENT_TG'] < 16777215)`.
|
|
- The comparison happens before `int(...)`, so string config values such as
|
|
`"9"` can be compared directly to integer `16777215`, raising `TypeError`.
|
|
- Numeric config values avoid the type error, but the expression becomes
|
|
`int(True)` or `int(False)`, so the second half of the range check is only
|
|
`1` or `0`.
|
|
- `bytes_3(CONFIG['SYSTEMS'][system]['OVERRIDE_IDENT_TG'])` is then called with
|
|
the raw config value rather than the parsed integer.
|
|
|
|
### Assumptions
|
|
|
|
- `OVERRIDE_IDENT_TG` is intended to direct voice ident packets to a configured
|
|
TG instead of all-call.
|
|
- Valid override TGs should be positive and less than the DMR all-call value
|
|
`16777215`.
|
|
- Empty string, `False`, or `0` should mean no override and should fall back to
|
|
all-call.
|
|
- Invalid override values should not crash `ident()`; they should fall back to
|
|
all-call and ideally be logged.
|
|
- Local/control TGs should be rejected for ident override.
|
|
|
|
### Unresolved Questions
|
|
|
|
- Should invalid startup `OVERRIDE_IDENT_TG` be normalized in runtime config, or
|
|
only ignored at ident send time?
|
|
- Resolved: invalid ident override values should be logged.
|
|
|
|
### Protocol-Sensitive Areas
|
|
|
|
- Voice ident sends generated DMR packets and chooses the destination TG for
|
|
those packets.
|
|
- All-call `16777215` is the fallback destination when no override is active.
|
|
- This path must not mutate packet bytes outside the intended generated voice
|
|
packet destination.
|
|
|
|
### Inferred Invariants
|
|
|
|
- Voice ident destination selection should parse and validate the override TG
|
|
before packet generation.
|
|
- Invalid override TG values should not prevent voice ident from running.
|
|
- Empty or disabled override values should use all-call.
|
|
|
|
### Resolution
|
|
|
|
- `OVERRIDE_IDENT_TG` is parsed before use.
|
|
- Valid positive TGs below all-call are used as the generated voice ident
|
|
destination.
|
|
- Empty string, `False`, and `0` use all-call.
|
|
- Malformed, out-of-range, or local/control TG values are logged and use
|
|
all-call.
|
|
|
|
## Current Analysis: Invalid TIMER Still Used Raw In Static TG Branches
|
|
|
|
### Findings
|
|
|
|
- `options_config()` now parses `DEFAULT_UA_TIMER` / `TIMER` into
|
|
`_default_ua_timer` and falls back to the current effective timer when the
|
|
option is malformed.
|
|
- Later in the same function, the TS1 and TS2 static TG update branches still
|
|
use `int(_options['DEFAULT_UA_TIMER'])` instead of `_default_ua_timer`.
|
|
- A client option string such as `TIMER=A;TS1=91` logs that the timer is invalid,
|
|
but then can still raise `ValueError` when the static TG branch runs.
|
|
- That aborts otherwise valid static TG changes in the same options string.
|
|
|
|
### Assumptions
|
|
|
|
- Invalid `TIMER` should be ignored independently for the session and should use
|
|
the current effective `DEFAULT_UA_TIMER`.
|
|
- Valid static TG changes in the same options string should still apply.
|
|
- The raw `DEFAULT_UA_TIMER` option value should not be converted again after
|
|
`_default_ua_timer` has been parsed and normalized.
|
|
|
|
### Unresolved Questions
|
|
|
|
- None for the immediate fix if the previously confirmed tolerant options model
|
|
applies.
|
|
|
|
### Protocol-Sensitive Areas
|
|
|
|
- The timer value controls bridge timeout state but should not block static TG
|
|
routing updates when malformed.
|
|
- Static TG updates must use the effective parsed timer, not raw client text.
|
|
|
|
### Inferred Invariants
|
|
|
|
- Once an option field is parsed into an effective value, subsequent bridge
|
|
mutations should use that effective value.
|
|
- Invalid independent option fields should not trigger `ValueError` as normal
|
|
control flow.
|
|
- Invalid option fields should be logged.
|
|
|
|
### Resolution
|
|
|
|
- TS1 and TS2 static TG updates now use the parsed effective `_default_ua_timer`
|
|
rather than the raw option string.
|
|
- Invalid `TIMER` values are logged and valid static TG updates in the same
|
|
options string still apply with the current effective timer.
|
|
|
|
### Clarified Configuration Model
|
|
|
|
- Client-supplied options are session-scoped. They should persist only for the
|
|
duration of that client session.
|
|
- FreeDMR has a disconnect/timeout routine that resets session-scoped options to
|
|
system configured defaults.
|
|
- Startup config-file defaults and client options are different layers and
|
|
should not be treated identically.
|
|
- If a startup config directive has an invalid field and that directive has a
|
|
defined system fallback default, FreeDMR should use the fallback as if the
|
|
directive had not been present and log that the startup default setting was
|
|
invalid.
|
|
- If a startup config directive is invalid and has no system default fallback,
|
|
FreeDMR should exit with an error.
|
|
- For client options, invalid fields should not become persisted configuration;
|
|
they should apply only if valid for the current session.
|
|
- Rejected configuration or client-option errors should be logged. Normalizing,
|
|
ignoring, or falling back silently is not acceptable because many rejected
|
|
options have no direct user feedback path.
|
|
|
|
### Startup Fallback Mapping Observed In `config.py`
|
|
|
|
- `DEFAULT_UA_TIMER` has a startup fallback of `10`.
|
|
- `SINGLE_MODE` has a startup fallback of `True`.
|
|
- `VOICE_IDENT` has a startup fallback of `True`.
|
|
- `TS1_STATIC` has a startup fallback of empty string.
|
|
- `TS2_STATIC` has a startup fallback of empty string.
|
|
- Empty `TS1_STATIC` and `TS2_STATIC` values are valid and mean no static TGs
|
|
for that slot.
|
|
- `OVERRIDE_IDENT_TG` has a startup fallback of `False`.
|
|
- `DEFAULT_REFLECTOR` is read with `getint()` and no fallback in `config.py`;
|
|
however the runtime policy now treats invalid positive values as disabled
|
|
default reflector state for the current session.
|
|
- Empty string, integer `0`, and boolean false for `DEFAULT_REFLECTOR` are
|
|
equivalent and mean no default reflector is set.
|
|
- `OPTIONS` is session-scoped and is reset by the HBP connection lifecycle to
|
|
`_default_options` when present, or removed otherwise.
|
|
|
|
## Voice Prompt Packet System Scoping
|
|
|
|
### Findings
|
|
|
|
- `sendVoicePacket(self, pkt, _source_id, _dest_id, _slot)` indexes
|
|
`systems[system].STATUS`, but `system` is not a function argument or local
|
|
variable.
|
|
- `sendSpeech(self, speech)` also indexes `systems[system].STATUS[2]` without a
|
|
local `system`.
|
|
- The call sites pass a router instance as `self`, for example
|
|
`reactor.callInThread(sendSpeech, self, speech)` and
|
|
`reactor.callFromThread(sendVoicePacket, systems[system], ...)`.
|
|
- In imported deterministic tests this can raise `NameError` if those functions
|
|
execute directly. In the full process it can be worse: module-level `for
|
|
system in ...` loops may leave a stale global `system`, causing prompt stream
|
|
state to be written to the wrong router.
|
|
|
|
### Assumptions To Validate
|
|
|
|
- Generated voice packets should update the status for the router instance being
|
|
sent on, i.e. `self._system`.
|
|
- Dial-a-TG voice prompts, disconnected announcements, on-demand files, and
|
|
idents should all be emitted on TG9 slot 2 unless their existing call path
|
|
intentionally selects a different destination.
|
|
- A voice prompt should not mutate stream status for another master just because
|
|
a global loop variable currently names that system.
|
|
|
|
### Unresolved Questions
|
|
|
|
- Should invalid prompt packet/state errors be caught and logged inside the
|
|
thread helpers, or should existing Twisted thread exception logging remain the
|
|
only failure report?
|
|
|
|
### Protocol-Sensitive Areas
|
|
|
|
- Voice prompt packet bytes and destination TG must remain unchanged unless the
|
|
production code intentionally generates them that way.
|
|
- The fix should be transport/state scoping only; it should not alter dial-a-TG
|
|
routing decisions or prompt content.
|
|
|
|
### Inferred Invariants
|
|
|
|
- Helper functions that receive a router instance should derive the system name
|
|
from `self._system`, not from a module-global loop variable.
|
|
- Prompt stream lifecycle bookkeeping must be attached to the same system that
|
|
sends the prompt packet.
|
|
|
|
### Resolution
|
|
|
|
- `sendVoicePacket()` and `sendSpeech()` now bind `system = self._system` before
|
|
accessing `systems[system].STATUS`.
|
|
- Deterministic coverage exercises `sendSpeech()` with a deliberately stale
|
|
module-level `system` value and verifies stream state and captured prompt
|
|
packets stay on the router instance's own system.
|
|
|
|
## Bridge Reset System Replacement
|
|
|
|
### Findings
|
|
|
|
- `remove_bridge_system(remsystem)` loops `for system in CONFIG['SYSTEMS']` and
|
|
rebuilds every bridge once per configured system.
|
|
- When it finds an entry where `bridgesystem['SYSTEM'] == remsystem`, it appends
|
|
a replacement entry with `'SYSTEM': system`, using the outer loop variable
|
|
rather than `remsystem`.
|
|
- Because `bt[bridge]` is overwritten on every outer-loop pass, the final bridge
|
|
state depends on the last configured system. In a two-master topology,
|
|
removing `MASTER-A` can leave two `MASTER-B` entries and no `MASTER-A` entry.
|
|
- Additional deterministic probes show the same identity corruption with three
|
|
masters and with OpenBridge present: the replacement entry can become
|
|
`MASTER-C` or even an `OBP-*` system depending on config iteration order.
|
|
- Neighboring reset helpers do not support this as intentional behavior:
|
|
`reset_static_tg()` and `reset_all_reflector_system()` preserve the target
|
|
system identity in replacement entries.
|
|
|
|
### Assumptions To Validate
|
|
|
|
- Bridge reset for a disconnected or timed-out master should keep that master's
|
|
bridge entry present but inactive, preserving its slot, TGID, timeout and
|
|
normal dial-a-TG activation trigger.
|
|
- Bridge reset should not create duplicate entries for another master.
|
|
- Bridge reset should not remove or deactivate unrelated systems' bridge entries.
|
|
|
|
### Unresolved Questions
|
|
|
|
- Should reset entries keep the previous `ON` list exactly as-is, or should they
|
|
keep the current behavior of setting `ON` to the entry's `TGID`?
|
|
|
|
### Protocol-Sensitive Areas
|
|
|
|
- This affects bridge state only; packet bytes should not be changed.
|
|
- Incorrect reset state can change which systems receive later TG traffic and
|
|
can prevent the reset master from reactivating the intended TG.
|
|
|
|
### Inferred Invariants
|
|
|
|
- A reset of system `X` must leave bridge entries for system `X` named `X`.
|
|
- The number of entries for unrelated systems should not change during reset.
|
|
|
|
### Resolution
|
|
|
|
- `remove_bridge_system()` now rebuilds each bridge once instead of once per
|
|
configured system.
|
|
- Replacement entries preserve `SYSTEM` as `remsystem`, set `ACTIVE` false, set
|
|
`TO_TYPE` to `ON`, reset the timer, and preserve existing `ON`, `OFF`, and
|
|
`RESET` trigger lists.
|
|
- Deterministic coverage verifies reset identity preservation for normal bridges
|
|
and trigger preservation for `#` reflector bridges with an FBP peer present.
|
|
|
|
## HBP Packet Reset/Reload Guard
|
|
|
|
### Findings
|
|
|
|
- `routerHBP.dmrd_received()` checks reset/reload state before parsing a packet:
|
|
`CONFIG['SYSTEMS'][self._system]['_reset'] or
|
|
CONFIG['SYSTEMS'][_system]['_reloadoptions']`.
|
|
- `_system` is assigned later in the same function by bridge iteration loops,
|
|
so Python treats it as a local variable. Referencing it in the guard before
|
|
assignment raises `UnboundLocalError` when the `_reset` key exists and does
|
|
not short-circuit safely.
|
|
- The `return` is outside the `if` block but inside the `try`. If both
|
|
`_reset` and `_reloadoptions` existed and were false, the function would still
|
|
return and drop the packet.
|
|
- The current code only reaches normal packet parsing when a `KeyError` is
|
|
raised by missing state keys. That makes normal operation depend on absent
|
|
lifecycle keys rather than their boolean values.
|
|
|
|
### Assumptions To Validate
|
|
|
|
- HBP packets should be dropped only while the receiving system's `_reset` or
|
|
`_reloadoptions` flag is true.
|
|
- The guard should inspect `CONFIG['SYSTEMS'][self._system]` for both flags.
|
|
- When a packet is dropped due reset/reload, it should be logged once using
|
|
`_resetlog`, preserving the current intent.
|
|
- When both flags are false or absent, packet handling should continue normally.
|
|
|
|
### Unresolved Questions
|
|
|
|
- Should reset and reload use separate one-shot log flags, or is the existing
|
|
shared `_resetlog` flag sufficient?
|
|
|
|
### Protocol-Sensitive Areas
|
|
|
|
- This is a packet admission guard. The fix should not mutate packet bytes or
|
|
alter routing after a packet is admitted.
|
|
- Incorrectly dropping during false reset/reload state can look like a silent
|
|
repeater or TG routing failure.
|
|
|
|
### Inferred Invariants
|
|
|
|
- Lifecycle flags should be treated as optional booleans with false defaults.
|
|
- A packet should be rejected during reset/reload only when the relevant flag is
|
|
explicitly true.
|
|
- Rejected packet admission during reset/reload should remain logged.
|
|
|
|
### Resolution
|
|
|
|
- The HBP packet admission guard now reads `_reset` and `_reloadoptions` from
|
|
`CONFIG['SYSTEMS'][self._system]` with false-default `dict.get()`.
|
|
- The guard returns only when reset or reload is active, and logs the rejection
|
|
once through `_resetlog`.
|
|
- Deterministic coverage verifies false lifecycle flags admit packets, while
|
|
active reset or reload drops packets and logs exactly once.
|
|
|
|
## HBP Unit Data To OBP Reporting Target
|
|
|
|
### Findings
|
|
|
|
- `routerHBP.sendDataToOBP()` sends the packet to `systems[_target]` correctly.
|
|
- When reporting is enabled, it then calls
|
|
`systems[system]._report.send_bridgeEvent(...)`, but `system` is not a
|
|
function argument or local variable in `sendDataToOBP()`.
|
|
- In a deterministic HBP unit-data-to-OBP path with reporting enabled, the
|
|
packet is captured for the OBP target and then the function raises
|
|
`NameError: name 'system' is not defined`.
|
|
- If a module-level `system` happens to exist in the full process, the report
|
|
event could be attached to the wrong system's report object instead of raising.
|
|
|
|
### Assumptions To Validate
|
|
|
|
- The TX report for `sendDataToOBP()` should be emitted on the target OBP
|
|
system's report object, matching the packet destination and the event payload.
|
|
- Reporting should not be able to raise after a packet send and interrupt the
|
|
rest of packet handling.
|
|
- This fix is reporting-only and should not change packet bytes, routing, stream
|
|
state, or source-quench behavior.
|
|
|
|
### Unresolved Questions
|
|
|
|
- None for the minimal fix; this appears to be a direct variable-name error.
|
|
|
|
### Protocol-Sensitive Areas
|
|
|
|
- The packet send path already uses `_target`; only the reporting object should
|
|
change.
|
|
- The event payload already names `_target`, so preserving it avoids report
|
|
semantic changes.
|
|
|
|
### Inferred Invariants
|
|
|
|
- Reporting side effects should use the same target system as the packet send
|
|
unless a caller explicitly reports on the source router.
|
|
- Enabling `CONFIG['REPORTS']['REPORT']` must not change packet routing
|
|
behavior or raise after a successful send.
|
|
|
|
### Resolution
|
|
|
|
- `routerHBP.sendDataToOBP()` now reports through `systems[_target]._report`,
|
|
matching the packet send target and existing event payload.
|
|
- Deterministic coverage verifies HBP unit data forwarded to OBP captures the
|
|
packet on the OBP target and records the TX report on that target report
|
|
object without raising.
|
|
|
|
### Data Packet Review Scope
|
|
|
|
- Per user direction, continue evaluating data packet processing before moving
|
|
to voice packet path debugging.
|
|
|
|
## OBP Data Gateway Metadata Argument Order
|
|
|
|
### Findings
|
|
|
|
- In `routerOBP.dmrd_received()`, the DATA-GATEWAY forwarding path calls
|
|
`sendDataToOBP('DATA-GATEWAY', ..., _source_rptr, _ber, _rssi)`.
|
|
- `routerOBP.sendDataToOBP()` expects positional arguments after `_slot` as
|
|
`_hops, _source_server, _ber, _rssi, _source_rptr`.
|
|
- This shifts metadata fields:
|
|
`_source_rptr` is sent as hops, `_ber` is sent as source server, `_rssi` is
|
|
sent as BER, RSSI falls back to zero, and source repeater falls back to zero.
|
|
- A deterministic probe with distinctive metadata captured:
|
|
`hops=b'\x01\x02\x03\x05'`, `source_server=b'\x12'`,
|
|
`ber=b'4'`, `source_rptr=b'\x00\x00\x00\x00'` for the DATA-GATEWAY target.
|
|
|
|
### Assumptions To Validate
|
|
|
|
- DATA-GATEWAY forwarding from OBP should preserve incoming `_hops`,
|
|
`_source_server`, `_ber`, `_rssi`, and `_source_rptr` exactly as received
|
|
unless production code intentionally rewrites them.
|
|
- The DATA-GATEWAY path should match the later OBP-to-OBP forwarding call that
|
|
already passes `_hops, _source_server, _ber, _rssi`.
|
|
- Because this is DATA-GATEWAY forwarding, preserving source repeater metadata is
|
|
preferable to falling back to zeros.
|
|
|
|
### Unresolved Questions
|
|
|
|
- Should `routerOBP.sendDataToOBP()` calls always use keyword arguments for
|
|
protocol metadata to prevent this class of positional-order bug?
|
|
|
|
### Protocol-Sensitive Areas
|
|
|
|
- This affects OBP metadata, not DMR payload bytes. The packet bytes should
|
|
remain unchanged except for existing slot-bit handling.
|
|
- Hops, source server, source repeater, BER, and RSSI can affect loop control,
|
|
observability and downstream data handling.
|
|
|
|
### Inferred Invariants
|
|
|
|
- Transport metadata forwarding must not be confused with protocol mutation.
|
|
- DATA-GATEWAY forwarding should not corrupt metadata by positional argument
|
|
shifts.
|
|
|
|
### Validation Result
|
|
|
|
- Rejected as a bug. User confirmed DATA-GATEWAY forwarding targets SMS/GPS
|
|
packet processing by KF1EEL and is protocol v1, not FBP.
|
|
- Protocol metadata expectations depend on protocol version; DATA-GATEWAY should
|
|
not be evaluated as though it were an FBP peer.
|
|
- Do not change this DATA-GATEWAY call based on FBP metadata preservation
|
|
assumptions.
|
|
- General protocol invariant confirmed by user: protocol options and argument
|
|
order must match the protocol version actually in use for that session.
|
|
|
|
## Unit Data To HBP Reporting Slot
|
|
|
|
### Findings
|
|
|
|
- Both `routerOBP.sendDataToHBP()` and `routerHBP.sendDataToHBP()` accept a
|
|
target slot argument named `_d_slot`.
|
|
- Callers calculate `_tmp_bits` before calling the helper, so the packet bytes
|
|
are rewritten to the target slot when needed.
|
|
- The helpers log and report the TX event with a hardcoded slot value of `1`.
|
|
- Deterministic probes show the packet is captured on slot 2 when SUB_MAP points
|
|
to slot 2, but the report event says slot 1:
|
|
`UNIT DATA,DATA,TX,MASTER-B,...,1,1234567`.
|
|
- This reproduces for both HBP-originated unit data and OBP-originated unit data
|
|
forwarded to an HBP master via `SUB_MAP`.
|
|
|
|
### Assumptions To Validate
|
|
|
|
- Unit data TX reports should identify the actual target HBP slot `_d_slot`.
|
|
- The packet bytes should remain unchanged; the existing caller-side slot bit
|
|
rewrite is already producing the expected target slot.
|
|
- This is reporting/observability only and should not alter data routing,
|
|
DATA-GATEWAY behavior, or protocol-version-specific metadata.
|
|
|
|
### Unresolved Questions
|
|
|
|
- Should the debug log wording also be changed from "on slot 1" to include
|
|
`_d_slot`, or should only the reporting payload change?
|
|
|
|
### Protocol-Sensitive Areas
|
|
|
|
- This is independent of OBP protocol version because the affected target is
|
|
HBP and `_d_slot` is already an explicit HBP target slot.
|
|
- Reporting should reflect the actual transport target without mutating packet
|
|
payload bytes.
|
|
|
|
### Inferred Invariants
|
|
|
|
- If a helper receives `_d_slot`, reporting and logs should not hardcode slot 1.
|
|
- Data packet tests should distinguish correct packet slot bits from incorrect
|
|
report metadata.
|
|
|
|
### Resolution
|
|
|
|
- `routerOBP.sendDataToHBP()` and `routerHBP.sendDataToHBP()` now use `_d_slot`
|
|
in both debug logging and `UNIT DATA,DATA,TX` report payloads.
|
|
- Deterministic coverage verifies HBP-originated and OBP-originated unit data
|
|
forwarded to HBP slot 2 via `SUB_MAP` captures slot 2 packet bits and reports
|
|
slot 2.
|
|
|
|
## OBP Unit CSBK Data Stream Classification
|
|
|
|
### Findings
|
|
|
|
- HBP unit data classification treats `_dtype_vseq == 3` as data when the packet
|
|
stream differs from the slot's current `RX_STREAM_ID`.
|
|
- HBP unit data handling does not update the slot `RX_STREAM_ID` in the data
|
|
branch, so repeated unit CSBK packets with the same stream continue through
|
|
the data forwarding path.
|
|
- OBP unit data classification treats `_dtype_vseq == 3` as data only when
|
|
`_stream_id not in self.STATUS`.
|
|
- Once the first OBP unit CSBK packet creates `self.STATUS[_stream_id]`,
|
|
subsequent unit CSBK packets with the same stream do not enter the data path
|
|
and are not forwarded via `SUB_MAP`.
|
|
- Deterministic probe: two HBP unit CSBK packets with the same stream both
|
|
forward to the HBP `SUB_MAP` target; two OBP unit CSBK packets with the same
|
|
stream forward only the first packet.
|
|
|
|
### Assumptions To Validate
|
|
|
|
- Repeated unit CSBK packets with the same stream ID should still be treated as
|
|
data packets and forwarded, unless duplicate control rejects the exact packet.
|
|
- OBP and HBP should not diverge on this classification solely because OBP uses
|
|
per-stream `STATUS` and HBP uses per-slot `STATUS`.
|
|
- If this is intentional first-packet-only OBP behavior for a protocol version,
|
|
the deterministic harness should document it rather than change it.
|
|
|
|
### Unresolved Questions
|
|
|
|
- Is `_dtype_vseq == 3` intended to represent only an initial unit CSBK in OBP,
|
|
or can a valid data transaction contain multiple non-identical CSBK packets
|
|
with the same stream ID?
|
|
|
|
### Protocol-Sensitive Areas
|
|
|
|
- This affects data packet admission to the forwarding path, not packet byte
|
|
mutation.
|
|
- Duplicate suppression should remain separate from protocol classification:
|
|
exact duplicate packets may be dropped, but distinct CSBK data packets should
|
|
not be lost just because their stream already exists if the protocol allows
|
|
them.
|
|
|
|
### Inferred Invariants
|
|
|
|
- Data packet classification should be explicit about whether stream state is a
|
|
first-packet guard or only duplicate/loop-control state.
|
|
- OBP/HBP behavioral differences need protocol-version justification.
|
|
|
|
### Deeper Analysis
|
|
|
|
- ETSI TS 102 361-2 describes CSBK preamble use as a way to improve successful
|
|
delivery to mobile stations that are scanning or using sleep mode for battery
|
|
saving.
|
|
- User confirmed a DMR terminal can send more than one CSBK, usually as a
|
|
preamble to wake battery-operated terminals, but only one is required for the
|
|
route.
|
|
- In FreeDMR's OBP path, the first unit CSBK creates per-stream `STATUS` and
|
|
can be forwarded through DATA-GATEWAY, other OpenBridge/FBP peers, `SUB_MAP`,
|
|
or hotspot matching.
|
|
- Subsequent data header/block packets (`_dtype_vseq` 6, 7, 8) are still always
|
|
classified as unit data for the same stream. Therefore, the OBP first-CSBK
|
|
gate does not block the actual data payload path.
|
|
- Forwarding only the first CSBK may be intentional: it is enough to establish
|
|
routing/wake-up behavior and avoids multiplying preamble bursts across the
|
|
network.
|
|
- HBP's different behavior appears partly caused by its per-slot data state: the
|
|
unit-data branch does not set `RX_STREAM_ID`, so repeated CSBKs continue to
|
|
satisfy `_stream_id != self.STATUS[_slot]['RX_STREAM_ID']`.
|
|
|
|
### Validation Result
|
|
|
|
- Do not change this yet. The observed OBP/HBP difference is real, but not yet a
|
|
confirmed bug.
|
|
- Treat OBP first-CSBK-only forwarding as likely intentional until a recorded
|
|
fixture or live test shows a valid data transaction needing multiple distinct
|
|
CSBKs forwarded over OBP for correct behavior.
|
|
- Add future fixture coverage before changing this classification logic.
|
|
|
|
### Follow-Up Verification
|
|
|
|
- Code parsing in `hblink.py` extracts `_stream_id = _data[16:20]` for every
|
|
DMRD packet before passing it into `dmrd_received()`.
|
|
- FreeDMR does not appear to model a single transaction-level stream ID for the
|
|
whole SMS/GPS data exchange. Each DMRD packet carries its own stream ID, and
|
|
data routing decisions are made per received packet.
|
|
- OBP unit data handling always classifies `_dtype_vseq` 6, 7 and 8 as data
|
|
regardless of whether `_stream_id` already exists, so data header/block packets
|
|
with distinct stream IDs are naturally handled independently.
|
|
- OBP unit CSBK handling classifies `_dtype_vseq == 3` as data only when that
|
|
packet's `_stream_id` is not already present in `self.STATUS`.
|
|
- Deterministic probe: two OBP unit CSBK packets with distinct stream IDs both
|
|
forward via `SUB_MAP`; two with the same stream ID only forward the first.
|
|
- This supports the interpretation that repeated CSBK preambles are expected to
|
|
be distinct packets with distinct stream IDs in this code path, and the
|
|
existing OBP first-CSBK-per-stream gate is likely a duplicate/preamble
|
|
suppression mechanism rather than a transaction-level routing bug.
|
|
- User confirmed the underlying protocol model: data packets are packet-oriented
|
|
and are not a stream in the same way AMBE2+ audio is a stream.
|
|
- User also confirmed DMR data is not always unit-to-unit; the standard permits
|
|
data packets to be sent as group calls to a talkgroup.
|
|
|
|
## HBP Group-Addressed Data Activates TG Bridge State
|
|
|
|
### Findings
|
|
|
|
- HBP `routerHBP.dmrd_received()` handles `group` and `vcsbk` packets in the
|
|
group-call path.
|
|
- For a new group packet, it logs `_dtype_vseq == 6` as `DATA HEADER`, but then
|
|
runs the same unknown-TG bridge creation block used for voice-like group calls:
|
|
`make_single_bridge(_dst_id, self._system, _slot, DEFAULT_UA_TIMER)`.
|
|
- Deterministic probe: a single HBP group data header to TG123 creates bridge
|
|
`123` with `MASTER-A` slot 2 active.
|
|
- If bridge `123` already exists and is active, the same group data header is
|
|
routed to target systems, so group-addressed data routing through existing TG
|
|
bridges appears intentional or at least supported.
|
|
- OBP group data to TG123 does not create a bridge in the same probe.
|
|
|
|
### Assumptions To Validate
|
|
|
|
- Group-addressed data packets may legitimately route over existing active TG
|
|
bridge state.
|
|
- Unknown group-addressed data packets should not necessarily create new
|
|
user-activated voice/TG bridge state just because the TG is unknown.
|
|
- If unknown group data is intended to create a bridge, that should be treated as
|
|
an explicit design decision because it can activate TG state without voice.
|
|
|
|
### Unresolved Questions
|
|
|
|
- Should group-addressed data headers (`_call_type == 'group'` and
|
|
`_dtype_vseq == 6`) create user-activated bridges for unknown TGs, or should
|
|
automatic bridge creation be restricted to voice-like group calls?
|
|
- Should OBP and HBP differ here, or should both follow the same group-data
|
|
bridge-creation policy?
|
|
|
|
### Protocol-Sensitive Areas
|
|
|
|
- Data packets are packet-oriented, so stream-style bridge activation semantics
|
|
may not be appropriate.
|
|
- Group data can be a valid TG-addressed packet, but routing it over an existing
|
|
bridge is different from creating new active bridge state.
|
|
|
|
### Inferred Invariants
|
|
|
|
- Packet routing and bridge-state activation should be considered separately.
|
|
- A data packet should not create voice/TG lifecycle state unless that is an
|
|
explicit FreeDMR policy.
|
|
|
|
### Validation Result
|
|
|
|
- Leave unchanged for now. User believes this behavior is deliberate.
|
|
- Likely rationale: allow routing of GPS packets sent as group calls to a local
|
|
APRS bridge without exposing them to the entire network, because they are
|
|
local-to-server.
|
|
- Do not treat HBP unknown group-data bridge creation as a confirmed bug unless
|
|
a later fixture or operational test disproves this policy.
|
|
|
|
## Enhanced OBP Missing-Keepalive Gate
|
|
|
|
### Findings
|
|
|
|
- `kaReporting()` logs enhanced OpenBridge targets with no `_bcka` as not
|
|
sendable: `not sending to system ... as KeepAlive never seen`.
|
|
- `routerOBP.to_target()` applies that rule for OBP-originated group/voice
|
|
traffic: if `ENHANCED_OBP` is true and `_bcka` is missing or older than 60
|
|
seconds, the target is skipped.
|
|
- `routerHBP.to_target()` only skips enhanced OpenBridge targets when `_bcka`
|
|
exists and is stale; if `_bcka` is missing, HBP-originated group/voice traffic
|
|
is still forwarded.
|
|
- `routerOBP.sendDataToOBP()` and `routerHBP.sendDataToOBP()` have the same
|
|
lenient missing-`_bcka` check, so unit data can be forwarded to enhanced
|
|
OpenBridge targets that have never sent a keepalive.
|
|
- Deterministic probes confirmed:
|
|
HBP unit data -> enhanced OBP without `_bcka` sends one packet; HBP group
|
|
voice -> enhanced OBP without `_bcka` sends one packet; OBP unit data ->
|
|
enhanced OBP without `_bcka` sends one packet; OBP group voice -> enhanced
|
|
OBP without `_bcka` sends no packet.
|
|
|
|
### Assumptions To Validate
|
|
|
|
- For `ENHANCED_OBP` targets, missing `_bcka` means the peer has not completed
|
|
the enhanced keepalive requirement and should not receive traffic.
|
|
- The intended rule should be the same for HBP-originated and OBP-originated
|
|
traffic, and for voice-like group packets and unit data packets.
|
|
- Periodic `kaReporting()` warnings are sufficient operational logging for
|
|
missing/stale enhanced keepalive suppression; per-packet skip logging would be
|
|
noisy.
|
|
|
|
### Unresolved Questions
|
|
|
|
- Should the strict missing-or-stale keepalive gate be applied everywhere an
|
|
enhanced OpenBridge target is selected, or is there a deliberate exception for
|
|
HBP-originated traffic/data?
|
|
|
|
### Protocol-Sensitive Areas
|
|
|
|
- Enhanced OBP keepalive state is transport/session state, not DMR payload
|
|
mutation.
|
|
- Data packet forwarding must preserve packet bytes except intentional slot or
|
|
transport metadata rewrites.
|
|
|
|
### Inferred Invariants
|
|
|
|
- Enhanced OpenBridge targets should not receive traffic until the session has
|
|
valid recent keepalive state.
|
|
- The same sendability predicate should be used by voice and data paths unless
|
|
an explicit protocol-version rule says otherwise.
|
|
|
|
### Validation Result
|
|
|
|
- User confirmed the strict missing-or-stale `_bcka` gate is intended for
|
|
enhanced OBP targets. The inconsistency likely came from protocol development
|
|
and was missed.
|
|
- Applied the strict gate in `routerHBP.to_target()`,
|
|
`routerOBP.sendDataToOBP()`, and `routerHBP.sendDataToOBP()` so the data
|
|
helpers and HBP-originated voice path match the existing OBP-originated voice
|
|
behavior.
|
|
- Added deterministic coverage for HBP unit data with missing, stale and recent
|
|
keepalive; OBP unit data with missing keepalive; and HBP group voice with
|
|
missing keepalive.
|
|
|
|
### Remaining Risk
|
|
|
|
- The black-box UDP harness does not yet exercise enhanced OBP keepalive
|
|
negotiation end to end. Current coverage validates the in-process routing
|
|
predicate and packet capture behavior.
|
|
|
|
## Group-Addressed Data Reported As Group Voice
|
|
|
|
### Findings
|
|
|
|
- HBP `routerHBP.dmrd_received()` recognizes a group data header when
|
|
`_call_type == 'group'` and `_dtype_vseq == 6`; it logs `*DATA HEADER*` and
|
|
emits `DATA HEADER,DATA,RX`.
|
|
- The same HBP group data header then enters the normal group routing path. The
|
|
target-side `to_target()` methods emit `GROUP VOICE,START,TX` for a new target
|
|
stream regardless of `_dtype_vseq`.
|
|
- OBP `routerOBP.dmrd_received()` does not special-case group data header
|
|
receive reporting; a group data header is reported as
|
|
`GROUP VOICE,START,RX`.
|
|
- `stream_trimmer_loop()` later emits `GROUP VOICE,END,RX` and/or
|
|
`GROUP VOICE,END,TX` for timed-out group data state, because the timeout
|
|
reporter is keyed on stream/slot state and does not distinguish data headers
|
|
from voice streams.
|
|
- Deterministic probes with reporting enabled confirmed:
|
|
HBP group data to HBP target reports `DATA HEADER,DATA,RX` on the source, but
|
|
`GROUP VOICE,START,TX` on the target; after timeout the source gets
|
|
`GROUP VOICE,END,RX` and the target gets `GROUP VOICE,END,TX`.
|
|
- OBP group data reports `GROUP VOICE,START,RX` on the OBP source and
|
|
`GROUP VOICE,START,TX` on the target.
|
|
|
|
### Assumptions To Validate
|
|
|
|
- Group-addressed data routing may be intentional, but report events should not
|
|
identify data packets as voice calls.
|
|
- Consumers of bridge reports expect event names and categories to describe the
|
|
packet class accurately enough to avoid showing phantom voice calls.
|
|
- If a group data header creates temporary stream/slot state, timeout reporting
|
|
should either use data-specific end events or avoid voice end events for that
|
|
data-only state.
|
|
|
|
### Unresolved Questions
|
|
|
|
- What report event names should be used for group-addressed data TX and OBP RX?
|
|
Possible names include `GROUP DATA HEADER,DATA,TX/RX` or reusing
|
|
`DATA HEADER,DATA,TX/RX`.
|
|
- Should timeout cleanup emit a data-specific end event for group data, or
|
|
should data headers avoid creating reportable voice lifecycle state?
|
|
|
|
### Protocol-Sensitive Areas
|
|
|
|
- This is reporting and stream/slot observability, not packet forwarding or
|
|
payload mutation.
|
|
- Group-addressed data is valid DMR traffic and may intentionally route over
|
|
existing TG bridges.
|
|
|
|
### Inferred Invariants
|
|
|
|
- Routing a packet over a TG bridge does not imply the report event should be
|
|
`GROUP VOICE`.
|
|
- Voice lifecycle reports should only describe AMBE voice-like streams, not
|
|
packet-oriented data headers.
|
|
|
|
### Validation Result
|
|
|
|
- User agreed this should change, with the caveat that the dashboard may have
|
|
limitations and will need later compatibility testing.
|
|
- Added `group_data_event_name()` for report classification of group/vcsbk data
|
|
events.
|
|
- HBP and OBP group data headers now report `DATA HEADER,DATA,RX/TX` instead of
|
|
`GROUP VOICE,START,RX/TX`.
|
|
- HBP slot state and OBP stream state now track data-only report state so
|
|
`stream_trimmer_loop()` suppresses `GROUP VOICE,END,RX/TX` for data-only
|
|
timeouts.
|
|
- Deterministic coverage verifies HBP and OBP group data reporting does not
|
|
emit `GROUP VOICE` events, and that normal HBP group voice still emits voice
|
|
start/end lifecycle reports.
|
|
|
|
### Remaining Risk
|
|
|
|
- The dashboard is the primary report consumer and may assume the older
|
|
`GROUP VOICE` event names. Dashboard compatibility remains to be tested
|
|
outside this codebase.
|
|
|
|
## HBP VCSBK Data RX Duplicate Reports
|
|
|
|
### Findings
|
|
|
|
- HBP `routerHBP.dmrd_received()` handles `_call_type == 'vcsbk'` in the group
|
|
call/data path.
|
|
- On a new VCSBK stream, the first `_call_type != 'group'` branch logs `*VCSBK*`
|
|
and emits `OTHER DATA,DATA,RX`.
|
|
- Immediately afterwards, the dedicated VCSBK block handler emits a second,
|
|
more specific report for `_dtype_vseq == 7` or `_dtype_vseq == 8`:
|
|
`VCSBK 1/2 DATA BLOCK,DATA,RX` or `VCSBK 3/4 DATA BLOCK,DATA,RX`.
|
|
- Deterministic probes confirmed HBP VCSBK dtype 7 and dtype 8 each produce two
|
|
RX report events on the source system.
|
|
- The target-side report is already specific (`VCSBK 1/2 DATA BLOCK,DATA,TX` or
|
|
`VCSBK 3/4 DATA BLOCK,DATA,TX`) because `to_target()` uses
|
|
`group_data_event_name()`.
|
|
- OBP-originated VCSBK dtype 7 and dtype 8 now produce only the specific RX
|
|
event and the specific TX event.
|
|
|
|
### Assumptions To Validate
|
|
|
|
- `OTHER DATA,DATA,RX` is intended as a fallback for VCSBK/other data types that
|
|
do not have a more specific report name.
|
|
- HBP VCSBK dtype 7/8 should emit only the specific VCSBK block RX event, not
|
|
both a generic and specific data event.
|
|
- Removing the duplicate generic HBP source report is reporting-only and should
|
|
not alter routing, packet bytes, or bridge activation behavior.
|
|
|
|
### Unresolved Questions
|
|
|
|
- Should any other VCSBK dtype values continue to report as `OTHER DATA,DATA,RX`
|
|
on first receipt?
|
|
|
|
### Protocol-Sensitive Areas
|
|
|
|
- VCSBK data block classification is protocol/data reporting, not AMBE voice
|
|
lifecycle.
|
|
- This change would affect dashboard/report consumers but not DMR payload
|
|
forwarding.
|
|
|
|
### Inferred Invariants
|
|
|
|
- A single received packet should normally produce one RX report event for its
|
|
packet class.
|
|
- Specific data report names should take precedence over generic fallback names.
|
|
|
|
### Validation Result
|
|
|
|
- User agreed that specific VCSBK events should take precedence.
|
|
- HBP `OTHER DATA,DATA,RX` fallback now only emits when
|
|
`group_data_event_name()` has no specific event name.
|
|
- Deterministic coverage verifies HBP VCSBK dtype 7 emits the specific
|
|
`VCSBK 1/2 DATA BLOCK,DATA,RX/TX` events without a generic RX duplicate.
|
|
- Deterministic coverage also verifies an unknown VCSBK dtype still emits
|
|
`OTHER DATA,DATA,RX`.
|
|
|
|
## Unknown VCSBK Reported As Group Voice
|
|
|
|
### Findings
|
|
|
|
- Unknown HBP VCSBK data types currently emit `OTHER DATA,DATA,RX` on the
|
|
source, but because `group_data_event_name()` returns `None` for unknown VCSBK
|
|
values, the same packet is treated as voice-like for target and timeout
|
|
reporting.
|
|
- Deterministic probe for HBP `_call_type == 'vcsbk'` and `_dtype_vseq == 5`
|
|
produced source reports:
|
|
`OTHER DATA,DATA,RX` followed by `GROUP VOICE,END,RX` on timeout.
|
|
- The target for the same HBP packet produced `GROUP VOICE,START,TX` followed by
|
|
`GROUP VOICE,END,TX` on timeout.
|
|
- OBP-originated unknown VCSBK currently produces only `GROUP VOICE,START,RX` on
|
|
source and `GROUP VOICE,START,TX` on target, followed by voice timeout events.
|
|
- This is parallel to the group-data reporting issue, but applies to generic
|
|
VCSBK fallback data rather than known VCSBK block types.
|
|
|
|
### Assumptions To Validate
|
|
|
|
- `vcsbk` packets are data/control signalling for report classification, even
|
|
when `_dtype_vseq` is not one of the known block values currently named by the
|
|
code.
|
|
- Unknown VCSBK types should use `OTHER DATA,DATA,RX/TX` instead of any
|
|
`GROUP VOICE` lifecycle event.
|
|
- Timeout cleanup should suppress `GROUP VOICE,END,RX/TX` for unknown VCSBK
|
|
state just as it does for known group/vcsbk data.
|
|
|
|
### Unresolved Questions
|
|
|
|
- Are there any VCSBK `_dtype_vseq` values that dashboard/report consumers
|
|
intentionally expect to appear as `GROUP VOICE`?
|
|
|
|
### Protocol-Sensitive Areas
|
|
|
|
- VCSBK is a signalling/data classification issue; packet bytes and routing
|
|
should remain unchanged.
|
|
- Dashboard compatibility remains a risk for report event names.
|
|
|
|
### Inferred Invariants
|
|
|
|
- Unknown data/control packet types should fall back to generic data reporting,
|
|
not voice lifecycle reporting.
|
|
- `group_data_event_name()` currently doubles as both the event-name selector
|
|
and the data-vs-voice lifecycle classifier; unknown VCSBK exposes that
|
|
coupling.
|
|
|
|
### Specification Check
|
|
|
|
- ETSI TS 102 361-1 V2.6.1 separates voice burst format from data/control burst
|
|
format. Voice bursts carry vocoder payload plus either sync or embedded
|
|
signalling in the centre field.
|
|
- The embedded signalling defined for voice bursts is LC, RC, privacy-related
|
|
signalling, or null embedded message. This is not a CSBK/data continuation
|
|
Slot Type burst.
|
|
- Data/control bursts use the general data burst format and contain a Slot Type
|
|
PDU whose Data Type defines the 196 information bits.
|
|
- The Data Type table lists CSBK, Data Header, Rate 1/2 Data Continuation, and
|
|
Rate 3/4 Data Continuation as data/control burst types.
|
|
- Therefore, the packets currently classified by the HomeBrew bit parser as
|
|
`vcsbk`/data-sync data types should not be reported as part of a voice stream.
|
|
Unknown VCSBK data types should fall back to generic data/control reporting,
|
|
not `GROUP VOICE` lifecycle reporting.
|
|
|
|
### Validation Result
|
|
|
|
- User approved the fix after the specification check.
|
|
- Added `is_group_data_control()` to classify group/vcsbk data-control packets
|
|
independently from their specific report event name.
|
|
- Added `group_data_report_name()` so known group/vcsbk data uses the specific
|
|
event name and unknown VCSBK falls back to `OTHER DATA`.
|
|
- HBP and OBP unknown VCSBK now emit `OTHER DATA,DATA,RX/TX` and suppress
|
|
`GROUP VOICE` timeout lifecycle events.
|
|
- Deterministic coverage verifies HBP and OBP unknown VCSBK fallback reporting
|
|
produces no `GROUP VOICE` events.
|
|
|
|
## OBP Unit Data Loop-Control Zero-Division
|
|
|
|
### Findings
|
|
|
|
- `routerOBP.dmrd_received()` unit-data handling creates or updates
|
|
`self.STATUS[_stream_id]`, increments `packets`, then performs OBP
|
|
first-source loop-control selection immediately in the same receive path.
|
|
- If another OBP system has the earlier `1ST` timestamp for the same
|
|
`_stream_id` and `_dst_id`, the losing source enters the `self._system != fi`
|
|
branch.
|
|
- That branch calculates `call_duration = pkt_time -
|
|
self.STATUS[_stream_id]['START']`, initializes `packet_rate = 0`, but then
|
|
divides by `call_duration` whenever `packets` exists:
|
|
`packet_rate = self.STATUS[_stream_id]['packets'] / call_duration`.
|
|
- Deterministic probe: inject the same OBP unit data packet with the same stream
|
|
ID into `OBP-1`, then into `OBP-2` without advancing the harness clock. The
|
|
second injection raises `ZeroDivisionError` at the packet-rate calculation.
|
|
- The HBP/OBP group loop-control path has a similar unguarded packet-rate
|
|
calculation, but the immediate two-source group probe did not crash because
|
|
that path only reaches the calculation after the stream already exists on the
|
|
receiving OBP source.
|
|
|
|
### Assumptions To Validate
|
|
|
|
- A zero-duration packet-rate sample should be reported as `0` rather than
|
|
crashing packet processing.
|
|
- Loop-control should still ignore the later source and update `LAST` as it
|
|
does today.
|
|
- This is a logging/diagnostic calculation; guarding it should not change
|
|
routing, packet bytes, or first-source selection behavior.
|
|
|
|
### Unresolved Questions
|
|
|
|
- Should the same duration guard be applied to all packet-rate calculations in
|
|
loop-control/rate-drop paths, including the OBP group path and HBP group
|
|
rate-drop path, as a defensive cleanup?
|
|
|
|
### Protocol-Sensitive Areas
|
|
|
|
- This is loop-control diagnostics for data forwarding, not protocol mutation.
|
|
- The first-source `1ST` selection remains the source-quench/loop-control
|
|
behavior under review.
|
|
|
|
### Inferred Invariants
|
|
|
|
- Packet processing must not fail because multiple packets share the same
|
|
timestamp.
|
|
- Diagnostic packet-rate calculations must be optional and guarded.
|
|
|
|
### Validation Result
|
|
|
|
- User approved the zero-duration guard.
|
|
- Added `call_duration` truthiness checks before calculating diagnostic
|
|
`packet_rate` in OBP unit-data and OBP group loop-control ignore paths.
|
|
- Deterministic coverage reproduces the same-timestamp two-OBP unit-data case
|
|
and verifies the later source is loop-controlled without raising.
|
|
|
|
## OBP Group Packet Rate Drop Uses Absolute Start Time
|
|
|
|
### Findings
|
|
|
|
- `routerOBP.dmrd_received()` group/vcsbk packet-rate protection checks
|
|
`self.STATUS[_stream_id]['packets'] > 18` and then divides packet count by
|
|
`self.STATUS[_stream_id]['START']`.
|
|
- `START` is an absolute timestamp captured from `pkt_time`, not an elapsed
|
|
duration.
|
|
- With normal epoch-like times, `packets / START` is effectively zero, so the
|
|
`> 25` packet-rate threshold will not fire.
|
|
- HBP `routerHBP.dmrd_received()` has the analogous rate-drop check, but divides
|
|
by elapsed time: `pkt_time - self.STATUS[_slot]['RX_START']`.
|
|
- Deterministic probe: 19 OBP group packets over about 0.095 seconds did not
|
|
call `proxy_BadPeer()`. The equivalent HBP probe logged `RATE DROP` and set
|
|
the slot `LAST` field.
|
|
- `hblink.HBSYSTEM.proxy_BadPeer()` iterates `_peers` and emits `PRBL` proxy
|
|
blacklist packets for connected HBP client/repeater peers.
|
|
- `hotspot_proxy_v2.py` consumes `PRBL` by looking up the peer in `peerTrack`
|
|
and blacklisting the tracked client source host.
|
|
- `routerOBP` is an OpenBridge peer system, not a hotspot proxy client session;
|
|
it does not have a valid HBP `_peers` set for this purpose and should not use
|
|
proxy client blacklisting as its OBP/FBP packet-rate response.
|
|
- If the OBP elapsed-time divisor is corrected while leaving
|
|
`self.proxy_BadPeer()` in place, the rate-drop path is likely to fail at
|
|
runtime rather than cleanly dropping/controlling the offending OBP stream.
|
|
- User clarified that using the proxy as an IP-level block point can be
|
|
deliberate: the proxy can install blacklist entries into iptables, and a
|
|
malicious flood from one IP may need to be blocked for both client and OBP
|
|
traffic.
|
|
- The remaining concern is therefore not the policy of using the proxy as a
|
|
blanket block point; it is that `routerOBP` inherits from `OPENBRIDGE`, while
|
|
`proxy_BadPeer()` is only defined on `HBSYSTEM`, and OBP
|
|
`dmrd_received()` is not currently passed the source `_sockaddr`.
|
|
|
|
### Assumptions To Validate
|
|
|
|
- OBP group/vcsbk rate-drop should use elapsed stream duration:
|
|
`pkt_time - self.STATUS[_stream_id]['START']`.
|
|
- A zero or near-zero elapsed duration should not crash; if the threshold cannot
|
|
be evaluated safely, the packet should not be rate-dropped solely by the
|
|
diagnostic calculation.
|
|
- The existing OBP enforcement policy of asking the proxy to blacklist a flood
|
|
source may be intentional and operationally useful.
|
|
- The current OBP call site still needs validation because it calls an HBP-only
|
|
helper and does not have the inbound OBP source socket address in
|
|
`routerOBP.dmrd_received()`.
|
|
- The intended OBP/FBP response should be either local packet/stream drop only,
|
|
local drop plus enhanced-OBP source quench, or local drop plus a correctly
|
|
targeted proxy blacklist request.
|
|
|
|
### Unresolved Questions
|
|
|
|
- Should the OBP rate-drop response send enhanced-OBP BCSQ source quench when
|
|
`ENHANCED_OBP` is enabled?
|
|
- If proxy blacklisting is retained for OBP floods, should the blacklist target
|
|
be the validated inbound `_sockaddr`, the configured `TARGET_SOCK`, or a
|
|
separate proxy-control address?
|
|
- Should the threshold stay at the existing 25 packets/second over more than 18
|
|
packets as a crash-protection guard, or should it be configurable later?
|
|
|
|
### Protocol-Sensitive Areas
|
|
|
|
- This is packet-rate control and bad-peer handling, not packet mutation.
|
|
- Changing the divisor can make an existing protection effective, which may
|
|
alter operational behavior under bursty traffic.
|
|
- `proxy_BadPeer()` currently belongs to the HBP/hotspot proxy control plane.
|
|
Reusing the proxy as a shared IP block point may be valid, but OBP needs an
|
|
explicit, correctly targeted path rather than an HBP-only method call.
|
|
- BCSQ is an enhanced-OBP source-quench mechanism; using it here would be a
|
|
protocol-visible overload signal, not just local loop protection.
|
|
|
|
### Inferred Invariants
|
|
|
|
- Packet-rate thresholds must be calculated against elapsed stream duration, not
|
|
absolute timestamps.
|
|
- Rate-limit calculations must guard zero elapsed duration.
|
|
- OBP/FBP overload handling must not depend on HBP client proxy state.
|
|
|
|
### Status
|
|
|
|
- Deferred by user.
|
|
- Do not change this path yet. The proxy/IP-blocking intent may be deliberate
|
|
because the proxy can feed blacklisted IPs into iptables and may be a useful
|
|
shared block point for malicious client and OBP floods.
|
|
- Return later with a focused review of whether `routerOBP` can correctly reach
|
|
that proxy block path as implemented, and whether the rate calculation should
|
|
be repaired without changing the intended block policy.
|
|
|
|
## HBP Unit Data To FBP Drops BER/RSSI Metadata
|
|
|
|
### Findings
|
|
|
|
- `routerHBP.dmrd_received()` extracts `_ber = _data[53:54]` and
|
|
`_rssi = _data[54:55]` for inbound HBP packets.
|
|
- HBP group/voice forwarding to OpenBridge passes `_ber` and `_rssi` through
|
|
`routerHBP.to_target()` into `systems[_target['SYSTEM']].send_system(...)`.
|
|
- HBP unit-data forwarding uses `routerHBP.sendDataToOBP()`, whose signature
|
|
accepts `_ber` and `_rssi`, and then passes them into
|
|
`systems[_target].send_system(...)`.
|
|
- The two HBP unit-data call sites pass only `_source_rptr` as the positional
|
|
argument after `_slot`:
|
|
`self.sendDataToOBP(..., _bits, _slot, _source_rptr)`.
|
|
- Because of the function signature, that value is bound to `_hops`, while
|
|
`_ber` and `_rssi` use their default zero values.
|
|
- `routerHBP.sendDataToOBP()` ignores the `_hops` argument and resets
|
|
`_source_server` and `_source_rptr` internally, so the current extra
|
|
positional argument is effectively confusing but harmless. The BER/RSSI loss
|
|
is the observable issue.
|
|
- Deterministic probe with inbound HBP unit data carrying `ber=b'B'` and
|
|
`rssi=b'R'` captured an OBP send call with `ber=b'\x00'` and
|
|
`rssi=b'\x00'`.
|
|
|
|
### Assumptions To Validate
|
|
|
|
- For normal FBP/OpenBridge peer forwarding, HBP-originated unit-data packets
|
|
should preserve inbound BER/RSSI metadata just like HBP-originated group/voice
|
|
packets do.
|
|
- Passing BER/RSSI into `send_system()` is protocol-version safe because
|
|
`OPENBRIDGE.send_system()` decides what fields are encoded based on the target
|
|
session protocol version.
|
|
- DATA-GATEWAY remains protocol-version-specific. Passing BER/RSSI is harmless
|
|
for a v1 DATA-GATEWAY target because v1 packet construction will not encode
|
|
the FBP metadata fields.
|
|
|
|
### Unresolved Questions
|
|
|
|
- Should the fix touch both HBP unit-data call sites, including DATA-GATEWAY, or
|
|
only the normal FBP peer forwarding call?
|
|
- Should `routerHBP.sendDataToOBP()` drop the unused `_hops` parameter or should
|
|
we keep the signature stable and use keyword arguments at the call sites?
|
|
|
|
### Protocol-Sensitive Areas
|
|
|
|
- This is transport/session metadata, not DMR payload mutation.
|
|
- Protocol option order must continue to match the protocol version actually
|
|
used by the target OpenBridge session.
|
|
- DATA-GATEWAY is not FBP by design; changes must not reinterpret it as FBP.
|
|
|
|
### Inferred Invariants
|
|
|
|
- DMR payload bytes must be preserved unless intentionally rewritten.
|
|
- HBP BER/RSSI metadata should not be silently zeroed when forwarding to a
|
|
protocol version that can carry it.
|
|
- Positional metadata arguments are fragile in these paths; keyword arguments
|
|
are safer for reviewable fixes.
|
|
|
|
### Validation Result
|
|
|
|
- User confirmed this is intentional future consistency work that was not
|
|
completed in the data path.
|
|
- Severity is low because BER/RSSI metadata is not central to data routing, but
|
|
preserving it should be done for consistency with the voice/group path.
|
|
- User confirmed DATA-GATEWAY may or may not be obsolete, but should remain in
|
|
place for now.
|
|
- Updated the HBP unit-data DATA-GATEWAY and normal OpenBridge/FBP call sites to
|
|
pass `_ber` and `_rssi` as keyword arguments, avoiding the previous positional
|
|
`_source_rptr` confusion.
|
|
- Added deterministic coverage proving HBP unit data with nonzero BER/RSSI is
|
|
forwarded to an OpenBridge target with that send metadata preserved while the
|
|
captured DMRD packet bytes remain metadata-free at the deterministic capture
|
|
point.
|
|
|
|
## HBP Group/VCSBK Rate Drop Zero-Division
|
|
|
|
### Findings
|
|
|
|
- `routerHBP.dmrd_received()` handles group and VCSBK packets on a per-slot
|
|
stream state.
|
|
- On a new group/VCSBK stream it sets `self.STATUS[_slot]['RX_START'] =
|
|
pkt_time`.
|
|
- Later in the same branch, before duplicate/out-of-order filtering, the packet
|
|
rate guard checks:
|
|
`self.STATUS[_slot]['packets'] / (pkt_time - self.STATUS[_slot]['RX_START'])`.
|
|
- If many packets are processed with the same `pkt_time` as the stream start,
|
|
the elapsed duration is zero and this branch raises `ZeroDivisionError`.
|
|
- Deterministic probe: injecting 19 same-timestamp HBP group data headers on the
|
|
same stream raises `ZeroDivisionError: division by zero`.
|
|
- The final call-end packet-rate logging path already guards zero elapsed
|
|
duration before dividing, and the earlier OBP loop-control diagnostic path was
|
|
already fixed to do the same.
|
|
|
|
### Assumptions To Validate
|
|
|
|
- The HBP packet-rate guard is intended to protect FreeDMR from excessive packet
|
|
rate, including accidental loop/flood conditions, not to crash when packets
|
|
arrive inside one timestamp quantum.
|
|
- If elapsed duration is zero, rate cannot be evaluated safely; the packet
|
|
should continue through existing duplicate/drop logic rather than rate-drop
|
|
solely from an undefined calculation.
|
|
- The existing HBP rate-drop policy should remain unchanged when elapsed
|
|
duration is nonzero: more than 18 packets and more than 25 packets/second
|
|
logs `RATE DROP`, updates `LAST`, and returns.
|
|
|
|
### Unresolved Questions
|
|
|
|
- Should extremely small nonzero durations be clamped to a minimum interval, or
|
|
is a simple zero guard sufficient for now?
|
|
|
|
### Protocol-Sensitive Areas
|
|
|
|
- This is local overload protection, not DMR payload mutation.
|
|
- HBP traffic has a practical slot-cadence limit, but test harnesses and bursty
|
|
runtime scheduling can still produce same-timestamp packet processing.
|
|
|
|
### Inferred Invariants
|
|
|
|
- Packet-rate calculations must never divide by zero.
|
|
- Existing HBP rate-drop behavior should be preserved for measurable elapsed
|
|
durations.
|
|
- Duplicate/out-of-order filtering should still run when rate cannot be
|
|
computed safely.
|
|
|
|
### Validation Result
|
|
|
|
- User approved the zero-duration guard.
|
|
- Added a `call_duration` guard to the HBP group/VCSBK rate-drop calculation.
|
|
- Added deterministic coverage for 19 same-timestamp HBP group data headers on
|
|
one stream; the path no longer raises and stream state remains intact.
|
|
|
|
## OBP Unit Data To FBP v5+ Drops Source Repeater Metadata
|
|
|
|
### Findings
|
|
|
|
- `routerOBP.sendDataToOBP()` accepts `_source_rptr` and passes it to the target
|
|
OpenBridge `send_system()` call:
|
|
`systems[_target].send_system(..., _hops, _ber, _rssi, _source_server,
|
|
_source_rptr)`.
|
|
- `routerOBP.dmrd_received()` receives `_source_rptr` as decoded metadata from
|
|
`OPENBRIDGE.datagramReceived()` only for FBP protocol versions above 4.
|
|
- `OPENBRIDGE.send_system()` serializes source server for FBP v4 and above, but
|
|
serializes source repeater only when the target OpenBridge session version is
|
|
above 4.
|
|
- The normal OBP-to-OBP unit-data forwarding call for protocol versions greater
|
|
than 1 passes `_hops`, `_source_server`, `_ber`, and `_rssi`, but omits
|
|
`_source_rptr`.
|
|
- Source server is preserved by this path. The observed loss is source repeater
|
|
metadata for target sessions whose version can carry it.
|
|
- For target sessions with version 4 or lower, passing `_source_rptr` would not
|
|
change serialized packets because `OPENBRIDGE.send_system()` does not encode
|
|
source repeater for those versions.
|
|
- Deterministic probe: OBP-1 unit data to OBP-2 with
|
|
`hops=b'\x05'`, source server `7654321`, BER `b'B'`, RSSI `b'R'`, and source
|
|
repeater `1234567` captured the OBP-2 send metadata as:
|
|
hops preserved, source server preserved, BER/RSSI preserved, but
|
|
`source_rptr=b'\x00\x00\x00\x00'`.
|
|
|
|
### Assumptions To Validate
|
|
|
|
- For normal FBP/OpenBridge peer forwarding, OBP-originated unit-data packets
|
|
should preserve `_source_rptr` only when the target session protocol version
|
|
supports it. Passing it into `send_system()` is expected to be safe because
|
|
`send_system()` already gates serialization by target `VER`.
|
|
- This is distinct from the DATA-GATEWAY path. DATA-GATEWAY remains a
|
|
protocol-v1 SMS/GPS path and should not be evaluated as FBP metadata
|
|
forwarding.
|
|
- Using keyword arguments for the normal OBP-to-OBP unit-data forwarding call is
|
|
the smallest safe fix and avoids repeating positional metadata mistakes.
|
|
|
|
### Unresolved Questions
|
|
|
|
- Should DATA-GATEWAY remain unchanged exactly as currently implemented, despite
|
|
its positional argument oddity, until a separate protocol-v1 review is done?
|
|
|
|
### Protocol-Sensitive Areas
|
|
|
|
- Source repeater is transport/session metadata, not DMR payload mutation.
|
|
- Protocol options and order must match the protocol version in use for the
|
|
target session.
|
|
- FBP metadata preservation should not imply DATA-GATEWAY is FBP.
|
|
- OBP v1 and lower FBP versions do not support source repeater metadata.
|
|
|
|
### Inferred Invariants
|
|
|
|
- Normal FBP v5+ forwarding should preserve source repeater metadata it receives
|
|
unless production code intentionally rewrites it.
|
|
- Source server preservation is already present in this unit-data path.
|
|
- DATA-GATEWAY protocol-v1 behavior should remain isolated from FBP metadata
|
|
expectations.
|
|
|
|
### Validation Result
|
|
|
|
- User confirmed this makes sense for unit data only.
|
|
- Updated the normal OBP-to-OBP unit-data forwarding call to pass
|
|
`_source_rptr` through to `routerOBP.sendDataToOBP()`.
|
|
- Left DATA-GATEWAY unchanged.
|
|
- Added deterministic coverage proving OBP-originated unit data forwarded to
|
|
another FBP peer preserves source server, source repeater, hops, BER and RSSI
|
|
send metadata.
|
|
|
|
## OpenBridge DMRE Parser Accepts Truncated Packets Into Indexing
|
|
|
|
### Findings
|
|
|
|
- `OPENBRIDGE.datagramReceived()` handles `DMRE` packets by reading
|
|
`_packet[55]` to choose the embedded protocol layout.
|
|
- It then indexes fixed offsets for v5+ (`_packet[72]`, `_packet[73:89]`) or
|
|
v4 (`_packet[68]`, `_packet[69:85]`) without first checking that the datagram
|
|
is long enough for that layout.
|
|
- Deterministic parser-seam probe: injecting `b"DMRE"` into an OpenBridge system
|
|
raises `IndexError: index out of range` at `_packet[55]`.
|
|
- A truncated `DMRE` packet long enough to contain byte 55 but shorter than the
|
|
full v5+ or v4 layout also raises `IndexError` when later fixed offsets are
|
|
accessed.
|
|
- Truncated `DMRD` v1 packets do not raise in the same simple probe because the
|
|
code computes an HMAC over the available bytes and rejects them before
|
|
indexing decoded fields. The observed crash is specific to `DMRE` parsing.
|
|
|
|
### Assumptions To Validate
|
|
|
|
- Malformed/truncated OpenBridge/FBP UDP packets should be logged and discarded,
|
|
not allowed to raise out of `datagramReceived()`.
|
|
- The correct minimum lengths are the layouts already implied by
|
|
`OPENBRIDGE.send_system()`: 89 bytes for v5+ `DMRE`, 85 bytes for v4 `DMRE`.
|
|
- This should be implemented as a parser guard only; it should not change
|
|
routing, metadata order, HMAC/BLAKE2 verification, or packet mutation for
|
|
valid packets.
|
|
|
|
### Unresolved Questions
|
|
|
|
- Should short-packet logs be warning-level like existing HMAC failures, or
|
|
debug-level to avoid noisy logs under scanning/flood attempts?
|
|
|
|
### Protocol-Sensitive Areas
|
|
|
|
- This is the raw UDP/parser seam in `hblink.py`, before `bridge_master.py`
|
|
decoded packet handling.
|
|
- Length guards must respect the active embedded protocol version. OBP v1,
|
|
FBP v4, and FBP v5+ have different packet layouts.
|
|
|
|
### Inferred Invariants
|
|
|
|
- Parser code should validate minimum length before fixed-offset indexing.
|
|
- Invalid transport packets should be rejected before decoded packet handling.
|
|
- Valid packet bytes and metadata must remain unchanged.
|
|
|
|
### Validation Result
|
|
|
|
- User approved adding parser guards.
|
|
- Added `DMRE` length checks in `OPENBRIDGE.datagramReceived()` before reading
|
|
byte 55, and before reading the fixed v5+ or v4 metadata offsets.
|
|
- Short `DMRE` packets are logged and discarded without reaching decoded packet
|
|
handling.
|
|
- Added deterministic parser-seam coverage for packets shorter than the version
|
|
byte and packets truncated after the version byte.
|
|
|
|
## HBP Master DMRD Parser Accepts Truncated Packets Into Indexing
|
|
|
|
### Findings
|
|
|
|
- `HBSYSTEM.master_datagramReceived()` handles `DMRD` packets by first slicing
|
|
`_peer_id = _data[11:15]` and validating that the peer is connected and the
|
|
source socket matches the tracked peer.
|
|
- After that peer/socket check passes, it indexes fixed fields:
|
|
`_seq = _data[4]`, `_bits = _data[15]`, and `_stream_id = _data[16:20]`.
|
|
- There is no minimum length check before those fixed-offset indexes.
|
|
- Deterministic parser-seam probe: register peer `1001` on `MASTER-A`, then
|
|
inject a 15-byte `DMRD` packet containing the matching peer ID at bytes
|
|
11..14. The parser raises `IndexError: index out of range` at `_data[15]`.
|
|
- This is not an arbitrary unauthenticated packet path in the reproduced case;
|
|
the packet must pass the tracked peer/socket check.
|
|
|
|
### Assumptions To Validate
|
|
|
|
- Malformed/truncated HBP `DMRD` packets from a connected peer should be logged
|
|
and discarded, not allowed to raise out of `master_datagramReceived()`.
|
|
- The minimum safe length for HBP `DMRD` parser indexing is 20 bytes, because
|
|
the parser reads through `_data[16:20]` and decoded packet handling expects
|
|
the 53-byte DMRD header/payload plus optional BER/RSSI later.
|
|
- A conservative 20-byte parser guard is enough to prevent fixed-offset parser
|
|
exceptions while preserving existing behavior for packets that reach decoded
|
|
packet handling.
|
|
|
|
### Unresolved Questions
|
|
|
|
- Should malformed-packet logging be rate-limited later if noisy connected peers
|
|
send repeated short packets?
|
|
|
|
### Protocol-Sensitive Areas
|
|
|
|
- This is HBP UDP parser admission, before `bridge_master.py` decoded packet
|
|
routing.
|
|
- Rejecting malformed packets must not change valid packet bytes, peer tracking
|
|
or authentication behavior.
|
|
|
|
### Inferred Invariants
|
|
|
|
- Parser code should validate minimum length before fixed-offset indexing.
|
|
- Invalid transport packets from connected peers should be rejected before
|
|
decoded packet handling.
|
|
|
|
### Validation Result
|
|
|
|
- User approved the parser guard.
|
|
- Added a 53-byte minimum length check for HBP `DMRD` packets in both
|
|
`master_datagramReceived()` and `peer_datagramReceived()`.
|
|
- Short HBP `DMRD` packets are logged and discarded before peer validation
|
|
proceeds into fixed-offset packet parsing.
|
|
- Added deterministic parser-seam coverage for a short `DMRD` packet from a
|
|
connected/tracked master peer.
|
|
|
|
## OpenBridge BCST STUN Receiver Hashes The Wrong Bytes
|
|
|
|
### Findings
|
|
|
|
- `OPENBRIDGE.send_bcst()` sends a Bridge Control STUN packet as:
|
|
`BCST + HMAC_SHA1(passphrase, BCST)`.
|
|
- `OPENBRIDGE.datagramReceived()` receives `BCST` by setting
|
|
`_hash = _packet[4:]`, then calculating
|
|
`HMAC_SHA1(passphrase, _packet[4:])`.
|
|
- That means the receiver compares `HMAC(BCST)` with `HMAC(HMAC(BCST))`, so a
|
|
valid packet generated by `send_bcst()` is rejected.
|
|
- Deterministic parser-seam probe: build `BCST + HMAC(passphrase, BCST)` and
|
|
inject it into an enhanced OpenBridge system. The packet logs
|
|
`BCST invalid STUN` and does not set `_STUN`.
|
|
- If the current receive check ever did pass, the trace log references `_tgid`
|
|
and `_stream_id`, but `BCST` does not define those variables in that branch.
|
|
The practical observed bug is the hash mismatch, which prevents the success
|
|
branch.
|
|
|
|
### Assumptions To Validate
|
|
|
|
- `BCST` receive validation should mirror `send_bcst()` and verify
|
|
`HMAC_SHA1(passphrase, BCST)`.
|
|
- A valid `BCST` should set the same stun state that the current success branch
|
|
intends to set.
|
|
- The success trace log should not reference TGID or stream ID because `BCST`
|
|
carries only the opcode plus HMAC.
|
|
|
|
### Unresolved Questions
|
|
|
|
- STUN currently has no observed timeout/clear path. It remains a conceptual
|
|
temporary quench mechanism and should be reviewed separately before relying on
|
|
it operationally.
|
|
|
|
### Protocol-Sensitive Areas
|
|
|
|
- This is enhanced OpenBridge bridge-control traffic, not DMR payload routing.
|
|
- The hash input must match the sender and must not accidentally change other
|
|
Bridge Control opcodes (`BCKA`, `BCSQ`, `BCVE`).
|
|
|
|
### Inferred Invariants
|
|
|
|
- Bridge Control receive validation should hash the same byte sequence the send
|
|
helper signs.
|
|
- STUN packets do not carry TGID or stream ID fields.
|
|
- A valid STUN request should set the `STUN` flag that existing OpenBridge
|
|
traffic gates already check.
|
|
|
|
### Validation Result
|
|
|
|
- User confirmed STUN was conceptual but requested making it internally
|
|
consistent. Intended concept: one server can tell another to temporarily stop
|
|
sending any FBP traffic.
|
|
- Updated `BCST` receive validation to verify `HMAC_SHA1(passphrase, BCST)`,
|
|
matching `send_bcst()`.
|
|
- Updated the success branch to log without TGID/stream ID fields.
|
|
- Updated the success branch to set `self._CONFIG['STUN'] = True`, matching the
|
|
existing send/receive stun gates that check for `STUN` in the global config.
|
|
- Added deterministic parser-seam coverage proving a valid generated `BCST`
|
|
sets the global `STUN` flag and no longer writes the unused `_STUN` key.
|
|
|
|
## OpenBridge BCSQ Target TGID Key Mismatch
|
|
|
|
### Findings
|
|
- `hblink.py`, `OPENBRIDGE.datagramReceived()`, `BCSQ` branch stores received
|
|
source-quench state in the OpenBridge peer system config as:
|
|
`_config['_bcsq'][_tgid] = _stream_id`.
|
|
- `bridge_master.py`, `routerOBP.to_target()`, checks that quench map with the
|
|
source packet destination `_dst_id`:
|
|
`(_dst_id in _target_system['_bcsq'])` and
|
|
`(_target_system['_bcsq'][_dst_id] == _stream_id)`.
|
|
- `bridge_master.py`, `routerHBP.to_target()`, uses inconsistent keys in the
|
|
equivalent check:
|
|
`(_dst_id in _target_system['_bcsq'])` but then indexes
|
|
`_target_system['_bcsq'][_target['TGID']]`.
|
|
- For same-TG bridge rules, `_dst_id == _target['TGID']`, so the bug is masked.
|
|
For cross-TG bridge rules, the membership test and lookup can disagree. If
|
|
`_dst_id` is present but `_target['TGID']` is not, the code can raise
|
|
`KeyError`; if only `_target['TGID']` is present, source quench is not applied.
|
|
|
|
### Assumptions
|
|
- BCSQ is intended to suppress further forwarding of a specific stream to the
|
|
OpenBridge system that sent the quench.
|
|
- User clarified that the only FreeDMR talkgroup rewrite is for dial-a-TG. In
|
|
that case, the HBP-side source is local TG9/TS2 and the OpenBridge target sees
|
|
the selected reflector TG.
|
|
- User clarified BCSQ semantics: source quench asks a peer to stop sending any
|
|
more packets for a given stream ID on a given TG. It is optional; failure to
|
|
quench is not fatal and mainly costs a small amount of bandwidth/processing.
|
|
- Therefore, the severity is lower than a routing/drop bug. The important
|
|
correctness property is that any local BCSQ check must compare the stream ID
|
|
and TGID in the same namespace as the BCSQ sender intended.
|
|
|
|
### Unresolved Questions
|
|
- Should the stream-trimmer cleanup remove BCSQ entries only after the matching
|
|
local stream expires, or should received BCSQ entries also have an independent
|
|
expiry? Current cleanup removes entries whose stored stream ID matches a
|
|
removed local OpenBridge stream.
|
|
|
|
### Protocol-Sensitive Areas
|
|
- `BCSQ` packet format is `BCSQ + TGID(3) + stream_id(4) + HMAC`.
|
|
- BCSQ semantics interact with dial-a-TG TG rewrite logic. The optional quench
|
|
may be ineffective if FreeDMR stores a quench under one TG namespace and
|
|
checks it under another.
|
|
|
|
### Inferred Invariants
|
|
- Source quench state must be checked with the same TGID namespace it is stored
|
|
under.
|
|
- Source quench is an optimization/control hint, not a required condition for
|
|
correctness of stream routing.
|
|
- A BCSQ match should drop forwarding only for the matching TGID and stream ID,
|
|
not all traffic to that OpenBridge target.
|
|
|
|
### Resolution
|
|
- User confirmed that for dial-a-TG, the BCSQ TGID should be the reflector TG,
|
|
not local TG9.
|
|
- Updated `bridge_master.py`, `routerHBP.to_target()`, so HBP-to-OpenBridge
|
|
source-quench checks test and index `_target_system['_bcsq']` with
|
|
`_target['TGID']`.
|
|
- Added deterministic dial-a-TG coverage proving a TG9/TS2 HBP reflector stream
|
|
is not forwarded to an FBP target when that target has source-quenched the
|
|
reflector TG and stream ID.
|
|
|
|
## OpenBridge STUN Has No Clear Or Expiry Path
|
|
|
|
### Findings
|
|
- `hblink.py`, `OPENBRIDGE.datagramReceived()`, `BCST` receive branch sets the
|
|
global config flag `self._CONFIG['STUN'] = True`.
|
|
- `hblink.py`, `OPENBRIDGE.send_system()`, `OPENBRIDGE.datagramReceived()` v1
|
|
`DMRD` path, and `OPENBRIDGE.datagramReceived()` `DMRE` path all gate traffic
|
|
with `if 'STUN' in self._CONFIG`.
|
|
- Repository-wide search finds no code path that removes `STUN`, changes it
|
|
back to false, or expires it by time.
|
|
- `BCST` has no duration field in the current packet format:
|
|
`BCST + HMAC_SHA1(passphrase, BCST)`.
|
|
- Current behavior after a valid `BCST` is therefore effectively permanent
|
|
until process restart or external mutation of the in-memory config.
|
|
|
|
### Assumptions To Validate
|
|
- User previously described the intended concept as one server asking another to
|
|
temporarily stop sending any FBP traffic.
|
|
- User clarified the likely intended design: a sysop/API operation would
|
|
un-stun the link. In that design, lack of automatic expiry is not necessarily
|
|
a bug; the missing piece is the operator/API clear path.
|
|
- Because `BCST` has no duration field, automatic expiry would be a local policy
|
|
change rather than protocol behavior.
|
|
- This should remain a global FBP traffic gate, not per-TG or per-stream; BCSQ
|
|
already covers the per-TG/per-stream source-quench case.
|
|
|
|
### Unresolved Questions
|
|
- Should the eventual sysop/API un-stun operation clear global `STUN` or a
|
|
per-link/per-peer stun state if the feature is later scoped more narrowly?
|
|
- Should STUN block only outbound FBP sends, or also inbound FBP processing? The
|
|
current gates block both outbound `send_system()` and inbound DMR packet
|
|
handling.
|
|
- Should the flag remain global across all OpenBridge systems, or should it be
|
|
scoped to the peer that sent `BCST`? The current code uses global
|
|
`self._CONFIG['STUN']`.
|
|
|
|
### Protocol-Sensitive Areas
|
|
- `BCST` is enhanced OpenBridge bridge-control traffic and currently carries no
|
|
duration or peer identity beyond the authenticated UDP source and configured
|
|
passphrase.
|
|
- Changing the traffic gate from a boolean/existence check to a timestamp must
|
|
preserve the current "block all FBP traffic while active" behavior if that is
|
|
confirmed.
|
|
|
|
### Inferred Invariants
|
|
- STUN is distinct from BCSQ: STUN is all FBP traffic, BCSQ is one stream on one
|
|
TG.
|
|
- Valid `BCST` should never mutate DMR payload bytes.
|
|
- If STUN is operator-cleared, the missing production behavior is an explicit
|
|
management/API clear operation, not an automatic timer.
|
|
|
|
## OBP Group Loop-Control Logs Duration As Packet Rate
|
|
|
|
### Findings
|
|
- `bridge_master.py`, `routerOBP.dmrd_received()`, group/vcsbk loop-control
|
|
branch calculates:
|
|
`call_duration = pkt_time - self.STATUS[_stream_id]['START']`.
|
|
- It then calculates a guarded `packet_rate`:
|
|
`packet_rate = self.STATUS[_stream_id]['packets'] / call_duration`.
|
|
- The debug log string says `PACKET RATE %0.2f/s`, but the argument passed is
|
|
`call_duration`, not `packet_rate`.
|
|
- The analogous OBP unit-data loop-control branch directly above passes
|
|
`packet_rate` correctly.
|
|
|
|
### Assumptions To Validate
|
|
- The log is intended to report packet rate, not duration, because the message
|
|
text says `PACKET RATE` and the code already computes `packet_rate`.
|
|
- This is a diagnostics-only bug. It does not affect packet routing, mutation,
|
|
source selection, BCSQ, or rate-drop enforcement.
|
|
- User clarified that data calls do not have a meaningful packet rate in this
|
|
sense because each packet is classed as its own stream; packet-rate assertions
|
|
should use voice stream fixtures, not data packet fixtures.
|
|
|
|
### Unresolved Questions
|
|
- No protocol question identified. This is local logging correctness.
|
|
|
|
### Protocol-Sensitive Areas
|
|
- None beyond avoiding any change to packet handling behavior.
|
|
|
|
### Inferred Invariants
|
|
- Diagnostic logs should report the metric named by the log message.
|
|
- Fixing the argument should not change any control-flow decisions.
|
|
|
|
### Resolution
|
|
- Updated the OBP group/vcsbk loop-control debug log to pass `packet_rate`
|
|
instead of `call_duration`.
|
|
- Added deterministic coverage that creates a two-second loop-controlled OBP
|
|
group voice stream and verifies the log reports the calculated `0.50/s`
|
|
packet rate, not the two-second duration as `2.00/s`.
|
|
|
|
## OBP Group Data RX Log Says Call Start
|
|
|
|
### Findings
|
|
- `bridge_master.py`, `routerOBP.dmrd_received()`, group/vcsbk new-stream branch
|
|
computes `_data_control = is_group_data_control(_call_type, _dtype_vseq)` and
|
|
reports data packets correctly with `DATA HEADER,DATA,RX`,
|
|
`VCSBK 1/2 DATA BLOCK,DATA,RX`, `VCSBK 3/4 DATA BLOCK,DATA,RX`, or
|
|
`OTHER DATA,DATA,RX`.
|
|
- The same branch always emits an info log labelled `*CALL START*` before the
|
|
report decision, even when `_data_control` is true.
|
|
- The HBP group path distinguishes group data header logging from voice start:
|
|
`_dtype_vseq == 6` logs `*DATA HEADER*`, while voice-like group packets log
|
|
`*CALL START*`.
|
|
- User clarified that data packets are packet-oriented and should not be treated
|
|
as voice streams for rate/lifecycle semantics.
|
|
|
|
### Assumptions To Validate
|
|
- OBP group data logs should match the already-correct report classification and
|
|
should not say `*CALL START*` for data-control packets.
|
|
- This is diagnostics/logging only; reports, routing, packet bytes, stream state
|
|
and lifecycle suppression are already handled separately.
|
|
|
|
### Unresolved Questions
|
|
- Should OBP group data logs include source server/hops metadata exactly like the
|
|
current `CALL START` log, or should they match the simpler HBP data-header log
|
|
shape?
|
|
|
|
### Protocol-Sensitive Areas
|
|
- This must not change `DATA_STREAM` classification, reporting events, packet
|
|
forwarding, or LC construction.
|
|
|
|
### Inferred Invariants
|
|
- Logs should not label DMR data-control packets as voice call lifecycle events.
|
|
- OBP and HBP logging should use consistent data-vs-voice terminology where the
|
|
packet classification is already known.
|
|
|
|
### Resolution
|
|
- User confirmed the change should be made, with the caveat that dashboard
|
|
consumers may be sensitive to live report/socket event text.
|
|
- Updated the OBP group/vcsbk RX info log to use the already-computed data event
|
|
label for data-control packets and `CALL START` only for voice-like packets.
|
|
- Added deterministic coverage proving an OBP group data header logs
|
|
`*DATA HEADER*` and no longer logs `*CALL START*`; existing report-socket
|
|
assertions still verify `DATA HEADER,DATA,RX/TX` payloads.
|
|
|
|
## Raw Parser May Not Classify VCSBK 3/4 Data Blocks
|
|
|
|
### Findings
|
|
- `bridge_master.py` has explicit group/vcsbk report handling for
|
|
`_call_type == 'vcsbk'` and `_dtype_vseq == 8`, labelled
|
|
`VCSBK 3/4 DATA BLOCK`.
|
|
- The raw packet parsers in `hblink.py` derive `_call_type` from the DMRD bits
|
|
with:
|
|
`elif (_bits & 0x23) == 0x23: _call_type = 'vcsbk'`.
|
|
- For a data-sync packet with `_dtype_vseq == 8`, the low nibble is `0x8`; with
|
|
`HBPF_DATA_SYNC` in bits 4..5, `_bits & 0x23` evaluates to `0x20`, not
|
|
`0x23`. That means the raw parser classifies it as `group`, not `vcsbk`.
|
|
- The deterministic in-process harness can inject `call_type='vcsbk'` directly,
|
|
so current VCSBK 3/4 report tests may cover `bridge_master.py` behavior without
|
|
proving a real UDP/HBP or UDP/OBP packet can reach that branch.
|
|
- Unit data handling is not affected by this specific observation because unit
|
|
data is classified first by the unit bit and then handles dtype 8 inside the
|
|
unit-data branch.
|
|
|
|
### Assumptions To Validate
|
|
- If FreeDMR expects group-addressed VCSBK 3/4 data blocks to arrive from real
|
|
HBP/OBP packets, the raw parser should classify the relevant bit pattern as
|
|
`vcsbk` so the existing report/routing classification can run.
|
|
- The parser predicate may have been intentionally written for only certain CSBK
|
|
or VCSBK bit patterns; changing it without confirming HomeBrew/DMR bit
|
|
semantics could misclassify normal group traffic.
|
|
- This is parser classification, not packet mutation.
|
|
- User raised an operational risk: earlier hackish data-over-OBP/FBP changes may
|
|
depend on this classification. A naive fix could affect whether data traverses
|
|
FBP or whether unknown group-addressed data creates local bridge state.
|
|
|
|
### Unresolved Questions
|
|
- Is `_dtype_vseq == 8` genuinely reachable for group/vcsbk packets in the
|
|
HomeBrew/OpenBridge bit layout, or is the `VCSBK 3/4 DATA BLOCK` branch legacy
|
|
or only relevant to direct/internal calls?
|
|
- Should the deterministic harness include a raw-datagram parser test for VCSBK
|
|
3/4 once the intended bit pattern is confirmed?
|
|
|
|
### Protocol-Sensitive Areas
|
|
- This is at the HBP/OBP raw DMRD parser seam in `hblink.py`.
|
|
- Incorrectly broadening the `vcsbk` predicate could change voice/group/data
|
|
classification before packets reach `bridge_master.py`.
|
|
- Existing active bridge forwarding likely still works for both `group` and
|
|
`vcsbk` because `bridge_master.py` routes both through the group/vcsbk path.
|
|
The more sensitive behavior is automatic unknown-TG bridge creation, which is
|
|
currently restricted to `_call_type == 'group'`.
|
|
|
|
### Inferred Invariants
|
|
- Parser-level `_call_type` classification should make production-reachable all
|
|
packet classes that `bridge_master.py` intentionally handles.
|
|
- In-process direct injection tests should not be mistaken for raw UDP parser
|
|
coverage when the question is bit-level classification.
|
|
- Any parser classification change must prove that existing data-over-FBP
|
|
traversal still works, including the local group-addressed data behavior
|
|
previously left intentional.
|
|
|
|
### Reclassification
|
|
- User clarified that the `unit`/`group` distinction is fundamentally the DMR
|
|
addressing mode: private/unit-unit bit set, or group/TG bit unset. Data/control
|
|
subtype is orthogonal to that addressing mode.
|
|
- Therefore, changing the raw parser to classify group-addressed data/control
|
|
packets as `vcsbk` would conflate addressing mode with data subtype and could
|
|
break intentional data-over-FBP behavior.
|
|
- Leave raw parser `_call_type` classification unchanged. Data/control handling
|
|
should be derived from `_frame_type` / `_dtype_vseq` helpers while preserving
|
|
group-vs-unit addressing.
|
|
|
|
## Group-Addressed Data Continuation Blocks Reported As Voice
|
|
|
|
### Findings
|
|
- `bridge_master.py`, `group_data_event_name()`, treats group-addressed
|
|
`_dtype_vseq == 6` as `DATA HEADER`.
|
|
- The same helper only treats `_dtype_vseq == 7` and `_dtype_vseq == 8` as data
|
|
when `_call_type == 'vcsbk'`.
|
|
- User clarified that `_call_type == 'group'` means group/TG addressing, not
|
|
voice. A group-addressed packet can still carry DMR data/control subtypes.
|
|
- Therefore, a group-addressed data continuation block that reaches
|
|
`bridge_master.py` as `_call_type == 'group'` and `_dtype_vseq == 7` or `8`
|
|
is currently classified as voice-like by `is_group_data_control()`.
|
|
- Consequences would match the earlier group-data report bug: RX/TX reports can
|
|
use `GROUP VOICE`, timeout cleanup can emit voice lifecycle events, and logs
|
|
can say `CALL START` for a packet-oriented data block.
|
|
|
|
### Assumptions To Validate
|
|
- Group-addressed data continuation blocks with dtype 7 or 8 should be classified
|
|
as data/control for reporting and timeout lifecycle, while preserving
|
|
`_call_type == 'group'` for TG-addressed routing and bridge creation behavior.
|
|
- Existing event labels `VCSBK 1/2 DATA BLOCK` and `VCSBK 3/4 DATA BLOCK` may be
|
|
legacy terminology. If the dashboard expects those names, changing labels could
|
|
be more disruptive than only broadening when they apply.
|
|
- This should not change raw parser classification or packet routing.
|
|
|
|
### Unresolved Questions
|
|
- Should dtype 7/8 group-addressed data use the existing
|
|
`VCSBK 1/2 DATA BLOCK` / `VCSBK 3/4 DATA BLOCK` report labels for dashboard
|
|
compatibility, or should labels be renamed to more generic data continuation
|
|
terminology later?
|
|
|
|
### Protocol-Sensitive Areas
|
|
- This is data-vs-voice reporting/lifecycle classification, not DMR payload
|
|
mutation.
|
|
- It must preserve the group-addressing behavior that lets data traverse
|
|
TG/FBP bridge paths.
|
|
|
|
### Inferred Invariants
|
|
- `_call_type == 'group'` should not imply voice.
|
|
- Data/control classification should be based on `_frame_type` and
|
|
`_dtype_vseq`, without breaking group-vs-unit addressing semantics.
|
|
|
|
### Resolution
|
|
- User approved broadening the helper-level classification.
|
|
- Updated `group_data_event_name()` so group-addressed dtype 7 and 8 packets use
|
|
the existing `VCSBK 1/2 DATA BLOCK` and `VCSBK 3/4 DATA BLOCK` data event
|
|
names.
|
|
- Raw parser `_call_type` classification remains unchanged; group-vs-unit
|
|
addressing is preserved.
|
|
- Added deterministic HBP and OBP coverage for group-addressed dtype 7/8 packets
|
|
proving they report as data and do not emit `GROUP VOICE` lifecycle events.
|
|
- Consolidated HBP known VCSBK RX reporting into the helper-driven new-stream
|
|
branch so specific VCSBK reports are still emitted once, without reintroducing
|
|
the older generic/specific duplicate report behavior.
|
|
|
|
## Voice Path: OBP Group Stream Rate Limit Uses Absolute Start Time
|
|
|
|
### Findings
|
|
- `bridge_master.py`, `routerOBP.dmrd_received()`, group/vcsbk stream duplicate
|
|
and rate-control branch, calculates the rate-drop predicate as
|
|
`self.STATUS[_stream_id]['packets'] / self.STATUS[_stream_id]['START'] > 25`.
|
|
- `START` is an absolute timestamp captured from `time()`, not elapsed stream
|
|
duration. In normal operation this divides by a large epoch value, so the
|
|
calculated rate is effectively zero.
|
|
- The surrounding code and earlier user clarification say this guard is intended
|
|
to stop packet floods from overwhelming FreeDMR. As written, it cannot trigger
|
|
for realistic packet counts.
|
|
- HBP group voice uses elapsed duration (`pkt_time - RX_START`) for the
|
|
comparable packet-rate guard.
|
|
|
|
### Assumptions To Validate
|
|
- The OBP/FBP guard should measure packets per elapsed stream duration, not
|
|
packets per absolute timestamp.
|
|
- The threshold should remain the existing `> 25` for now; changing policy is
|
|
separate from fixing the calculation.
|
|
- The existing `packets > 18` warm-up should remain, so short startup bursts are
|
|
not judged before enough packets have arrived.
|
|
- The deferred question about whether `proxy_BadPeer()` is the right response to
|
|
an OBP flood remains deferred; this analysis only covers the broken rate
|
|
calculation.
|
|
|
|
### Unresolved Questions
|
|
- Confirm whether this OBP rate-drop action should still call `proxy_BadPeer()`
|
|
after the calculation is corrected, or whether that should be revisited later
|
|
as previously deferred.
|
|
|
|
### Protocol-Sensitive Areas
|
|
- This is transport/rate-control behavior, not DMR payload parsing or mutation.
|
|
- OBP/FBP can carry multiple arbitrary streams, so this guard is only per stream
|
|
and does not represent an overall link-rate limit.
|
|
|
|
### Inferred Invariants
|
|
- Packet-rate protection should be based on elapsed time for the specific stream.
|
|
- Fixing the denominator should not change TG routing, slot handling, LC rewrite,
|
|
or packet bytes.
|
|
|
|
### Resolution
|
|
- User confirmed proceeding with the narrow denominator fix.
|
|
- Updated `routerOBP.dmrd_received()` so the group/vcsbk rate-drop check computes
|
|
`call_duration = pkt_time - self.STATUS[_stream_id]['START']` and divides
|
|
packet count by elapsed duration.
|
|
- Kept the existing `> 25` threshold, `packets > 18` warm-up, and
|
|
`proxy_BadPeer()` action unchanged.
|
|
- Added deterministic OBP group voice coverage proving the rate-drop path fires
|
|
for a high-rate stream when elapsed duration is short.
|
|
|
|
## Voice LC Rewrite Applied To Data-Sync Control Packets
|
|
|
|
### Findings
|
|
- `bridge_master.py`, `routerOBP.to_target()`, rewrites embedded LC for any
|
|
packet where `_dtype_vseq in [1,2,3,4]`.
|
|
- `bridge_master.py`, `routerHBP.to_target()`, has the same condition in both
|
|
the OpenBridge-target and HBP-target branches.
|
|
- These branches are intended for voice bursts B-E, but they do not check
|
|
`_frame_type`.
|
|
- A deterministic probe with an HBP-originated `vcsbk` packet using
|
|
`_frame_type == HBPF_DATA_SYNC` and `_dtype_vseq == 3` captured a forwarded
|
|
HBP packet whose DMR payload bytes were changed:
|
|
`000102030405060708090a0b0c0d00c122b4b2131415161718191a1b1c1d1e1f20`
|
|
instead of the original
|
|
`000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f20`.
|
|
- That mutation is the embedded-LC rewrite being applied to a data-sync/control
|
|
packet, not a voice burst.
|
|
- ETSI TS 102 361-1 distinguishes voice bursts containing embedded signalling
|
|
from data/control bursts. Search result snippets from the ETSI TS 102 361-1
|
|
PDF describe a voice burst containing embedded signalling and separately state
|
|
that data/control bursts use data SYNC or RC signalling rather than embedded
|
|
LC.
|
|
|
|
### Assumptions To Validate
|
|
- Embedded LC rewrite should only apply to real voice burst frames, not
|
|
data-sync/control packets.
|
|
- Voice header and voice terminator rewrite remain governed by
|
|
`_frame_type == HBPF_DATA_SYNC` plus `_dtype_vseq == HBPF_SLT_VHEAD` or
|
|
`HBPF_SLT_VTERM`, because those are voice call signalling bursts.
|
|
- VCSBK/CSBK/data-control packets should preserve their DMR payload bytes while
|
|
still allowing transport/header fields such as target TG and slot bit to be
|
|
rewritten by bridge routing when that is already intentional.
|
|
|
|
### Unresolved Questions
|
|
- Future work: evaluate carrying source embedded LC to the destination instead
|
|
of always regenerating it. Embedded LC may carry embedded GPS and talker alias
|
|
data, which FreeDMR does not currently relay.
|
|
|
|
### Protocol-Sensitive Areas
|
|
- This is a packet mutation bug at the boundary between voice LC rewrite and
|
|
data/control forwarding.
|
|
- The fix must not disable intended TG/LC rewrite for voice calls that are mapped
|
|
from one bridge TG to another.
|
|
- It must preserve the previously discussed data-over-FBP behavior and group-vs-
|
|
unit addressing classification.
|
|
|
|
### Inferred Invariants
|
|
- Data/control payload bytes are not embedded-LC containers and should not be
|
|
modified by voice LC rewrite code.
|
|
- Voice LC rewrite should be gated by both frame class and dtype/voice sequence.
|
|
|
|
### Resolution
|
|
- User confirmed embedded-LC rewrite should not apply to data-sync/control
|
|
packets.
|
|
- `mk_voice.py` shows generated voice uses `HBPF_VOICE_SYNC` for burst A with
|
|
vseq `0`, and `HBPF_VOICE` for bursts B-F; embedded LC is inserted only for
|
|
bursts B-E, vseq `1..4`.
|
|
- Updated all four embedded-LC rewrite branches in `routerOBP.to_target()` and
|
|
`routerHBP.to_target()` to require `_frame_type == HBPF_VOICE` and
|
|
`_dtype_vseq in [1,2,3,4]`.
|
|
- Added deterministic payload-preservation coverage for data-sync/control
|
|
packets across HBP-to-HBP, HBP-to-FBP, FBP-to-HBP and FBP-to-FBP forwarding.
|
|
|
|
## Private Unit Voice Timeout Reported As Group Voice
|
|
|
|
### Findings
|
|
- `bridge_master.py`, `routerHBP.dmrd_received()`, private AMI voice branch and
|
|
private dial-a-TG branch update slot RX state (`RX_TYPE`, `RX_TGID`,
|
|
`RX_TIME`, `RX_STREAM_ID`) for unit/private calls.
|
|
- Those private branches set `self.STATUS[_slot]['VOICE_STREAM'] = _voice_call`,
|
|
but `_voice_call` is initialized to `False` and is never set to `True`.
|
|
- `VOICE_STREAM` is not initialized in `routerHBP.__init__()` and is not read by
|
|
`stream_trimmer_loop()`.
|
|
- `stream_trimmer_loop()` emits `GROUP VOICE,END,RX` for any HBP slot whose
|
|
`RX_TYPE` is not terminator and whose `RX_DATA_STREAM` flag is false.
|
|
- Therefore a private dial-a-TG/AMI unit voice packet that does not receive a
|
|
terminator can be timed out as `GROUP VOICE,END,RX`, even though there was no
|
|
matching `GROUP VOICE,START,RX`.
|
|
- Deterministic probe: after 100 seconds of idle time, inject a private unit
|
|
voice call to `235`, advance 6 seconds, and run `stream_trimmer_loop()`. The
|
|
report event is:
|
|
`GROUP VOICE,END,RX,MASTER-A,16909060,1001,3120001,1,235,100.00`.
|
|
- The duration is also wrong for the private call because private branches do not
|
|
set `RX_START` on new private streams; the timeout report used the slot's old
|
|
initialization/start value.
|
|
|
|
### Assumptions To Validate
|
|
- Dial-a-TG and AMI private unit voice calls should not emit `GROUP VOICE`
|
|
lifecycle reports.
|
|
- HBP timeout reporting should emit `GROUP VOICE,END,RX` only for RX state that
|
|
originated from a group voice stream that would have emitted
|
|
`GROUP VOICE,START,RX`.
|
|
- Private unit calls may still mark the RF slot busy until timeout/terminator;
|
|
the issue is lifecycle reporting and duration classification, not slot
|
|
occupancy.
|
|
|
|
### Unresolved Questions
|
|
- Should private unit voice timeout produce a different report event, or should
|
|
it simply suppress `GROUP VOICE` timeout reporting?
|
|
- Should the existing `VOICE_STREAM` key be completed and used as the explicit
|
|
trimmer gate, or should a clearer `RX_VOICE_STREAM` / `RX_GROUP_VOICE_STREAM`
|
|
key be introduced?
|
|
|
|
### Protocol-Sensitive Areas
|
|
- This is report/lifecycle classification for HBP slot state, not packet
|
|
mutation.
|
|
- Dial-a-TG private control behavior and voice prompts should remain unchanged.
|
|
|
|
### Inferred Invariants
|
|
- `GROUP VOICE,END,RX` should correspond to an actual group voice RX lifecycle.
|
|
- Timeout cleanup should not infer group voice from `RX_TYPE != VTERM` alone.
|
|
|
|
### Resolution
|
|
- User approved correcting this even if the old behavior was a dashboard
|
|
workaround.
|
|
- Added explicit HBP slot state `RX_GROUP_VOICE_STREAM`, initialized false.
|
|
- Private AMI and dial-a-TG unit voice branches now mark
|
|
`RX_GROUP_VOICE_STREAM` false when they update slot RX state.
|
|
- The group/vcsbk branch marks `RX_GROUP_VOICE_STREAM` true only when the packet
|
|
is not data/control.
|
|
- `stream_trimmer_loop()` now emits `GROUP VOICE,END,RX` only when
|
|
`RX_GROUP_VOICE_STREAM` is true, then clears the flag with the timeout cleanup.
|
|
- Added deterministic coverage proving a private dial-a-TG unit voice timeout no
|
|
longer emits an unmatched `GROUP VOICE,END,RX` report.
|
|
|
|
## HBP-To-HBP Target TX State Uses Source RX Stream Predicate
|
|
|
|
### Findings
|
|
- `bridge_master.py`, `routerHBP.to_target()`, HBP-target branch, initializes
|
|
target TX state when `_stream_id != self.STATUS[_slot]['RX_STREAM_ID']`.
|
|
- That predicate describes whether the source HBP slot sees a new RX stream, not
|
|
whether the target HBP slot needs a new TX stream.
|
|
- The analogous `routerOBP.to_target()` HBP-target branch correctly checks the
|
|
target slot: `_target_status[_target['TS']]['TX_STREAM_ID'] != _stream_id`.
|
|
- Deterministic reproduction:
|
|
- MASTER-A sends stream `01020304` on TG123 to MASTER-B.
|
|
- After `STREAM_TO`, MASTER-C sends stream `01020305` on TG123 to MASTER-B.
|
|
- After another gap, MASTER-A sends another packet on stream `01020304`.
|
|
- The forwarded packet to MASTER-B carries stream `01020304`, but
|
|
MASTER-B's `TX_STREAM_ID` and `TX_RFS` remain `01020305` / MASTER-C.
|
|
- For a voice burst B-E packet, the forwarded DMR payload is rewritten using
|
|
MASTER-B's stale `TX_EMB_LC` generated for MASTER-C, not MASTER-A.
|
|
- This can create stale target lifecycle state, stale TX reports/durations, and
|
|
wrong embedded LC source information when a target slot has moved to another
|
|
stream while the original source later resumes the same stream id.
|
|
|
|
### Validated Assumptions
|
|
- User clarified this interleaved/resumed-stream scenario should never happen in
|
|
normal DMR operation. DMR channel access and hang-time mechanisms should
|
|
prevent an older stream from being resumed after a different stream has
|
|
interspersed on the same target path.
|
|
- Stream IDs are generated locally from inbound call data and are not expected to
|
|
be reused in this interleaved way.
|
|
|
|
### Unresolved Questions
|
|
- None for now. Leave this path unchanged unless a real fixture or live trace
|
|
proves valid stream interleaving occurs.
|
|
|
|
### Protocol-Sensitive Areas
|
|
- This affects target-side stream state and LC rewrite metadata for HBP targets.
|
|
- It does not change routing eligibility or packet source/destination header
|
|
fields.
|
|
|
|
### Inferred Invariants
|
|
- Target TX state should describe the packet currently being sent to that target.
|
|
- Source RX stream state and target TX stream state are related but independent.
|
|
- LC rewrite state must be regenerated when the target TX stream id changes.
|
|
|
|
### Resolution
|
|
- No runtime change. Treat the deterministic reproduction as an invalid scenario,
|
|
not a confirmed production bug.
|
|
- Keep the note as a protocol-sensitive area to revisit only if recorded traffic
|
|
shows valid resumed/interleaved stream behavior.
|
|
|
|
## OBP Target Timeout Reports Wrong Direction And TG After Dial-A-TG Rewrite
|
|
|
|
### Findings
|
|
- `bridge_master.py`, `routerHBP.to_target()`, OpenBridge-target branch creates
|
|
target stream state with `TGID: _dst_id`, even though the outbound packet and
|
|
`GROUP VOICE,START,TX` report use `_target['TGID']`.
|
|
- `bridge_master.py`, `routerOBP.to_target()`, OpenBridge-target branch does the
|
|
same for OBP-to-OBP targets.
|
|
- `stream_trimmer_loop()` handles all OpenBridge stream timeouts as
|
|
`GROUP VOICE,END,RX` and reports `_stream['TGID']`.
|
|
- Deterministic reproduction with dial-a-TG:
|
|
- Private call links reflector `#235`.
|
|
- HBP source sends group voice on local TS2 TG9, which is forwarded to FBP
|
|
target TG235.
|
|
- OBP target reports start as:
|
|
`GROUP VOICE,START,TX,OBP-1,16909060,1001,3120001,1,235`.
|
|
- If no terminator arrives and the OBP target stream times out, the report is:
|
|
`GROUP VOICE,END,RX,OBP-1,16909060,1001,3120001,1,9,0.00`.
|
|
- The timeout event is wrong in two ways for the target-side stream: direction is
|
|
`RX` instead of `TX`, and TG is local TG9 instead of the reflector TG235 that
|
|
was sent to the FBP peer.
|
|
- Immediate terminator handling in `to_target()` already emits
|
|
`GROUP VOICE,END,TX` with `_target['TGID']`; the mismatch only affects timeout
|
|
cleanup when no terminator is observed.
|
|
|
|
### Validated Assumptions
|
|
- User clarified that the report side intentionally reports HBP/RF-side TG9 for
|
|
dial-a-TG reflector traffic so the repeater dashboard shows TG9 active to RF.
|
|
- The FBP-visible reflector TG is not necessarily the desired TG for this
|
|
dashboard report path.
|
|
|
|
### Unresolved Questions
|
|
- There remains broader inconsistency between immediate terminator TX reports and
|
|
timeout RX reports for target-side FBP streams, but this is not currently
|
|
treated as a bug because the dashboard policy is intentional.
|
|
|
|
### Protocol-Sensitive Areas
|
|
- This is report/lifecycle state for OpenBridge/FBP target streams, not packet
|
|
routing or DMR payload mutation.
|
|
- BCSQ/loop-control logic also reads OpenBridge stream `TGID`; changing that key
|
|
directly could affect source-quench behavior. A separate report TG field may be
|
|
safer.
|
|
|
|
### Inferred Invariants
|
|
- For dial-a-TG dashboard reporting, RF-side TG9 activity is a first-class
|
|
observability requirement.
|
|
|
|
### Resolution
|
|
- No runtime change. Treat this as intentional reporting policy for dashboard
|
|
compatibility rather than a confirmed bug.
|
|
|
|
## Generated Voice First Packet Does Not Mark HBP TX Activity
|
|
|
|
### Findings
|
|
- `bridge_master.py`, `sendVoicePacket()`, handles generated voice packets for
|
|
dial-a-TG prompts, disconnected announcements, on-demand AMBE files and voice
|
|
idents.
|
|
- On the first packet of a generated stream, it creates
|
|
`systems[system].STATUS[_stream_id]` and sets `_slot['TX_TGID']`, but it does
|
|
not update `_slot['TX_TIME']`, `_slot['TX_TYPE']`, `_slot['TX_STREAM_ID']`,
|
|
`_slot['TX_START']`, or `_slot['TX_RFS']`.
|
|
- `_slot['TX_TIME']` is only updated in the `else` branch for subsequent packets.
|
|
- Deterministic probe after 100 seconds idle:
|
|
- Before first generated packet, TS2 state was
|
|
`TX_TYPE=HBPF_SLT_VTERM`, `TX_TIME=1700000000.0`,
|
|
`TX_STREAM_ID=b'\\x00'`, `TX_TGID=0`.
|
|
- After first generated packet, state was still
|
|
`TX_TYPE=HBPF_SLT_VTERM`, `TX_TIME=1700000000.0`,
|
|
`TX_STREAM_ID=b'\\x00'`, but `TX_TGID=9`.
|
|
- Because routing and idle checks use `TX_TIME` and `TX_TYPE`, a generated prompt
|
|
can look idle at stream start.
|
|
- Second deterministic probe: after sending the first generated prompt packet on
|
|
MASTER-B TS2 TG9, an inbound group voice packet from MASTER-A to MASTER-B was
|
|
still routed immediately to MASTER-B, producing overlapping outbound traffic.
|
|
|
|
### Assumptions To Validate
|
|
- User clarified that voice prompts should ideally be interruptible. At minimum,
|
|
after a prompt has finished, a real voice stream should not be immediately
|
|
blocked as "busy".
|
|
- Therefore, blindly updating `_slot['TX_TIME']` for generated prompt packets is
|
|
not an acceptable fix by itself because normal group-hangtime checks may treat
|
|
the prompt as recent TX activity and block post-prompt routing.
|
|
- Generated prompts should be treated as low-priority local traffic relative to
|
|
real RF/network voice.
|
|
|
|
### Unresolved Questions
|
|
- Should generated prompts/idents be represented as full HBP TX voice lifecycle
|
|
state for dashboard/report purposes, or should they only update `TX_TIME` to
|
|
protect the RF slot without creating extra report events?
|
|
- What exact interruption trigger should stop a prompt: any RX activity on that
|
|
HBP system/slot, any TX routing state change on that slot, or only a higher
|
|
priority group/private voice stream?
|
|
|
|
### Protocol-Sensitive Areas
|
|
- This is generated local voice scheduling/state, not inbound packet parsing.
|
|
- It affects collision avoidance between generated prompts and bridged traffic.
|
|
|
|
### Inferred Invariants
|
|
- A locally generated voice prompt must not permanently or hangtime-block
|
|
subsequent real voice traffic.
|
|
- Prompt collision avoidance should prefer stopping the prompt over blocking RF
|
|
or bridge traffic where practical.
|
|
- Report visibility for generated prompts is separate from collision avoidance.
|
|
|
|
### Late Entry Sanity Check
|
|
|
|
#### Findings
|
|
- ETSI TS 102 361-1 describes DMR late entry by carrying LC information in the
|
|
embedded field of voice bursts; a six-burst voice superframe uses one sync
|
|
burst, four embedded-signalling bursts, and one null/other embedded burst.
|
|
- `mk_voice.py` builds generated voice with `bptc.encode_emblc(LC)` and places
|
|
the LC fragments into bursts B-E. Burst A carries voice sync; burst F carries
|
|
null embedded LC.
|
|
- `bridge_master.py` builds `EMB_LC` / `TX_EMB_LC` for target streams and
|
|
rewrites only `HBPF_VOICE` bursts with vseq 1-4, corresponding to embedded LC
|
|
fragments B-E. The earlier guard now avoids mutating data-sync/control
|
|
payloads.
|
|
- Therefore, if a real voice stream interrupts a generated prompt and is then
|
|
forwarded through the normal routing path, receivers that missed the original
|
|
voice header should be able to late-enter once they receive a complete
|
|
embedded-LC sequence.
|
|
|
|
#### Assumptions
|
|
- Generated prompts are local low-priority streams and may be cancelled in favor
|
|
of real RF/network voice.
|
|
- Interrupting a prompt should resume normal bridge routing for the real stream;
|
|
it should not synthesize replacement voice frames outside the existing
|
|
header/embedded-LC machinery.
|
|
|
|
#### Unresolved Questions
|
|
- Whether prompt cancellation should send a terminator for the abandoned prompt
|
|
before real traffic starts. This may be cleaner semantically, but it could also
|
|
delay the higher-priority stream and defeat interruption.
|
|
- Whether generated prompt lifecycle state should be visible to dashboards as a
|
|
full TX stream, independent of collision avoidance.
|
|
|
|
#### Protocol-Sensitive Areas
|
|
- Late-entry behavior depends on preserving or correctly regenerating embedded LC
|
|
over bursts B-E. Any prompt-interruption fix must not remove the current
|
|
`EMB_LC` / `TX_EMB_LC` handling.
|
|
- If the first forwarded real packet is mid-superframe, receiving radios may
|
|
need to wait until the next complete embedded LC set before identifying the
|
|
call. This is expected late-entry behavior, not necessarily a bridge bug.
|
|
|
|
#### Inferred Invariants
|
|
- Real voice streams must remain the source of truth for LC/header state after a
|
|
prompt is cancelled.
|
|
- Prompt cancellation should stop local generated packet production; the
|
|
following real stream should use existing voice header and late-entry LC
|
|
processing.
|
|
|
|
### Resolution
|
|
- Added explicit generated-prompt state to HBP slot status:
|
|
`TX_PROMPT_ACTIVE`, `TX_PROMPT_CANCEL`, `TX_PROMPT_TOKEN`,
|
|
`TX_PROMPT_TIME`, `TX_PROMPT_STREAM_ID`, `TX_PROMPT_TGID` and
|
|
`TX_PROMPT_RFS`.
|
|
- `sendVoicePacket()` now records prompt activity from the first generated
|
|
packet without driving normal group-hangtime fields such as `TX_TIME` /
|
|
`TX_TYPE`.
|
|
- `sendSpeech()`, `disconnectedVoice()` and `playFileOnRequest()` now use a
|
|
per-slot prompt token and stop scheduling generated packets once the slot is
|
|
cancelled or taken over.
|
|
- HBP real group voice now cancels generated prompt state on the source slot for
|
|
local RX and on HBP target slots when a real voice stream is about to be
|
|
routed. Data/control packets do not cancel prompts.
|
|
- The real stream remains on the existing voice routing path, so voice headers
|
|
and B-E embedded LC rewrites continue to provide DMR late-entry behavior.
|
|
- Added deterministic tests for first-packet prompt state, prompt cancellation in
|
|
the speech loop, and real HBP voice cancelling a prompt while preserving
|
|
late-entry embedded LC rewrite.
|
|
|
|
## OBP Finished Voice Stream Depends On Reporting Being Enabled
|
|
|
|
### Findings
|
|
- `bridge_master.py`, `routerOBP.dmrd_received()`, has explicit finished-stream
|
|
handling: if `'_fin' in self.STATUS[_stream_id]`, later packets for the same
|
|
stream are ignored.
|
|
- The OBP voice terminator branch sets `self.STATUS[_stream_id]['_fin'] = True`
|
|
only inside `if CONFIG['REPORTS']['REPORT'] and not DATA_STREAM`.
|
|
- Deterministic probe:
|
|
- With `REPORT=False`, inject OBP voice header, terminator, then another voice
|
|
burst with the same stream ID. The HBP target capture count increases from 2
|
|
to 3, and `_fin` is absent.
|
|
- With `REPORT=True`, the same late packet is ignored; target capture remains
|
|
at 2, and `_fin` is present.
|
|
|
|
### Assumptions To Validate
|
|
- OBP stream lifecycle state should not depend on whether dashboard/live report
|
|
output is enabled.
|
|
- A voice terminator should mark the OBP stream finished for local loop-control
|
|
and late-packet suppression regardless of report configuration.
|
|
- The report event itself should remain gated by `CONFIG['REPORTS']['REPORT']`.
|
|
|
|
### Unresolved Questions
|
|
- Whether data/control packets can ever legitimately use `HBPF_SLT_VTERM` on the
|
|
OBP group path. Current code treats the branch as "voice terminator", so the
|
|
minimal fix should preserve the existing `not DATA_STREAM` guard for `_fin`.
|
|
|
|
### Protocol-Sensitive Areas
|
|
- This is stream lifecycle/loop-control state, not packet byte mutation.
|
|
- Changing `_fin` affects whether late packets with the same stream ID are
|
|
forwarded after a terminator.
|
|
|
|
### Inferred Invariants
|
|
- Runtime stream lifecycle must be independent from optional reporting.
|
|
- OBP finished-stream suppression is intended to be functional behavior, not a
|
|
dashboard side effect.
|
|
|
|
### Resolution
|
|
- Moved `self.STATUS[_stream_id]['_fin'] = True` out of the report-enabled block
|
|
while keeping it guarded by `not self.STATUS[_stream_id].get('DATA_STREAM')`.
|
|
- Kept `send_bridgeEvent('GROUP VOICE,END,RX,...')` inside the reporting gate.
|
|
- Added deterministic coverage that an OBP voice terminator suppresses later
|
|
packets with the same stream ID when reporting is disabled.
|
|
|
|
## HBP Voice Terminator Does Not Suppress Late Same-Stream Packets
|
|
|
|
### Findings
|
|
- `bridge_master.py`, `routerHBP.dmrd_received()`, handles HBP group voice
|
|
terminators by logging/reporting call end and resetting `lastSeq` /
|
|
`lastData`, then later updates slot state with `RX_TYPE = HBPF_SLT_VTERM` and
|
|
`RX_STREAM_ID = _stream_id`.
|
|
- Unlike the OBP path, HBP slot state has no finished-stream marker.
|
|
- A later HBP voice burst with the same stream ID after the terminator is not
|
|
rejected as finished. Because `RX_STREAM_ID` still matches, it is not treated
|
|
as a new stream, and because `lastSeq` was reset, duplicate/order checks do
|
|
not stop it.
|
|
- Deterministic probe:
|
|
- Inject HBP voice header, terminator, then another voice burst with the same
|
|
stream ID.
|
|
- The HBP target capture count increases from 2 to 3, meaning the post-
|
|
terminator voice burst is routed.
|
|
- Source slot `RX_TYPE` becomes the late packet's vseq (`3`) and `lastSeq`
|
|
becomes `3`, effectively reopening activity for the terminated stream.
|
|
|
|
### Assumptions To Validate
|
|
- A DMR voice terminator should close the voice stream for routing purposes.
|
|
- Late packets with the same stream ID after a valid voice terminator should be
|
|
suppressed, not routed or allowed to reopen slot activity.
|
|
- This should apply to voice streams only; group data/control classification
|
|
should keep its existing behavior.
|
|
|
|
### Unresolved Questions
|
|
- Whether there are any HBP implementations that can legitimately send useful
|
|
late voice bursts after a terminator with the same stream ID. The code's OBP
|
|
finished-stream guard suggests FreeDMR already treats such packets as invalid
|
|
on peer-server links.
|
|
- How long the HBP finished marker should live. A per-slot marker can probably be
|
|
cleared when the slot's `RX_STREAM_ID` is nulled by the existing 60-second
|
|
loop-control cleanup.
|
|
|
|
### Protocol-Sensitive Areas
|
|
- This is voice stream lifecycle and duplicate/late-packet suppression, not DMR
|
|
payload mutation.
|
|
- It affects only packets that arrive after a terminator for the same stream ID.
|
|
|
|
### Inferred Invariants
|
|
- A voice terminator is an end-of-stream signal.
|
|
- HBP and OBP should agree that post-terminator voice for the same stream ID is
|
|
finished traffic and should not be forwarded.
|
|
|
|
### Resolution
|
|
- Added per-slot `RX_FINISHED_STREAM_ID` and `RX_FINISHED_STREAM_LOG` state for
|
|
HBP systems.
|
|
- HBP voice terminators now mark the finished stream ID when the stream is not a
|
|
data/control stream.
|
|
- HBP group/vcsbk processing now ignores later voice-like packets whose stream
|
|
ID matches the finished marker, logging once for the suppressed late stream.
|
|
- The finished marker is cleared when the existing 60-second null-stream cleanup
|
|
clears `RX_STREAM_ID`, and when a genuine new voice stream begins.
|
|
- Added deterministic coverage proving a late HBP same-stream voice burst after
|
|
a terminator is suppressed.
|
|
|
|
### Live RF Validation Caveat
|
|
- Some voice-path behavior in FreeDMR may have been intentionally shaped around
|
|
real repeater/radio behavior, dashboard expectations, or interoperability
|
|
quirks that are not visible in the deterministic harness.
|
|
- DMR terminals may interpret the DMR standards loosely or incompletely,
|
|
especially around data packets. FreeDMR should generally prioritize preserving
|
|
usable audio flow over rigid protocol purity when those goals conflict.
|
|
- The HBP finished-stream suppression and generated-prompt interruption changes
|
|
are protocol-defensible and covered by deterministic tests, but they still
|
|
require live RF-path testing with a real terminal, repeater and network path.
|
|
- Be prepared to revert or narrow these changes if live RF testing shows that a
|
|
repeater, radio or dashboard depends on the previous behavior.
|
|
|
|
## OBP Target Terminator Leaves Stream Open For Timeout Report
|
|
|
|
### Findings
|
|
- `bridge_master.py`, `routerHBP.to_target()` and `routerOBP.to_target()`, create
|
|
per-stream status entries on OpenBridge targets when forwarding group voice.
|
|
- When a voice terminator is forwarded to an OpenBridge target, the target path
|
|
rewrites the terminator LC and, if reporting is enabled, emits
|
|
`GROUP VOICE,END,TX`.
|
|
- The OpenBridge target stream is not marked `_fin` on the forwarded terminator.
|
|
- `stream_trimmer_loop()` later treats that target-side OpenBridge stream as
|
|
stale and emits `GROUP VOICE,END,RX` for the same stream.
|
|
- Deterministic probes:
|
|
- HBP -> OBP: after header + terminator, OBP target reports
|
|
`GROUP VOICE,START,TX` and `GROUP VOICE,END,TX`; after six seconds, trimmer
|
|
adds `GROUP VOICE,END,RX` for the same stream.
|
|
- OBP -> OBP shows the same duplicate `END,RX` timeout after an immediate
|
|
target `END,TX`.
|
|
|
|
### Assumptions To Validate
|
|
- A forwarded voice terminator should close the target-side OpenBridge stream
|
|
lifecycle.
|
|
- Once an immediate target `GROUP VOICE,END,TX` has been emitted for a forwarded
|
|
terminator, cleanup should not later emit a timeout-style `GROUP VOICE,END,RX`
|
|
for the same target-side stream.
|
|
- This should be limited to non-data voice streams, matching the existing
|
|
`DATA_STREAM` guards.
|
|
|
|
### Validated Assumptions
|
|
- User confirmed the timer exists to terminate streams where the RF signal was
|
|
lost and the terminal's terminator was also lost.
|
|
- Therefore, timeout cleanup should cover missing terminators, not streams where
|
|
FreeDMR already forwarded a terminator.
|
|
|
|
### Protocol-Sensitive Areas
|
|
- This is target-side stream lifecycle/reporting, not packet routing or payload
|
|
mutation.
|
|
- It affects HBP -> OBP and OBP -> OBP forwarded voice terminators.
|
|
|
|
### Inferred Invariants
|
|
- OpenBridge target streams should be marked finished when FreeDMR forwards a
|
|
voice terminator to that target.
|
|
- Timeout cleanup should report missing terminators, not duplicate already-ended
|
|
streams that had a terminator.
|
|
|
|
### Resolution
|
|
- In both OpenBridge target branches, set `_target_status[_stream_id]['_fin'] =
|
|
True` when forwarding a non-data voice terminator.
|
|
- Kept existing `GROUP VOICE,END,TX` reporting behavior unchanged.
|
|
- Added deterministic tests for HBP -> OBP and OBP -> OBP proving the trimmer
|
|
does not emit a later duplicate `GROUP VOICE,END,RX` after a forwarded
|
|
terminator.
|
|
|
|
## OBP Source To OBP Target BCSQ TG Namespace
|
|
|
|
### Findings
|
|
- `bridge_master.py`, `routerOBP.to_target()`, OpenBridge target branch checks
|
|
source-quench state with `_dst_id`:
|
|
`(_dst_id in _target_system['_bcsq'])`.
|
|
- The same logical branch in `routerHBP.to_target()` checks
|
|
`_target['TGID']`, which is the TGID that will be visible to the target after
|
|
bridge rewrite.
|
|
- If an OBP source stream is bridged from one TG to a different OBP target TG,
|
|
a target BCSQ for the rewritten target TG is ignored.
|
|
- Deterministic probe:
|
|
- Bridge OBP-1 TG9 to OBP-2 TG235.
|
|
- Set OBP-2 `_bcsq` to `{235: stream_id}`.
|
|
- Inject OBP-1 stream on TG9.
|
|
- Packet still forwards to OBP-2 as TG235, proving the BCSQ check used the
|
|
wrong TG namespace.
|
|
|
|
### Validated Assumptions
|
|
- User clarified that OBP-to-OBP should not rewrite TGID; inbound OBP TG should
|
|
equal target OBP TG.
|
|
- The only expected TG rewrite is HBP <-> OBP when dial-a-TG maps local RF TG9
|
|
to an OBP/FBP reflector TG.
|
|
- BCSQ should use the TGID the source server sees and sends on OBP/FBP, not the
|
|
TG the client/terminal sees on RF. The quench asks the source server to stop
|
|
sending its traffic.
|
|
|
|
### Unresolved Questions
|
|
- Whether FreeDMR should defensively prevent or normalize OBP-to-OBP bridge
|
|
entries that imply TG rewrite. That is a bridge-configuration validity question
|
|
rather than a confirmed BCSQ runtime bug.
|
|
|
|
### Protocol-Sensitive Areas
|
|
- This is optional source-quench behavior. Failure does not corrupt packet bytes,
|
|
but it can waste bandwidth and processing by continuing to forward a quenched
|
|
stream.
|
|
- The TG namespace is source-server/OBP-visible, not RF/client-visible.
|
|
|
|
### Inferred Invariants
|
|
- OBP-source BCSQ matching should use the inbound OBP TGID and stream ID because
|
|
that is the source server's namespace.
|
|
- HBP-to-OBP dial-a-TG BCSQ matching should use the OBP/FBP reflector TG, not RF
|
|
TG9, because that is the TGID visible to the source server on OBP/FBP.
|
|
|
|
### Resolution
|
|
- No runtime change. The current OBP-source `_dst_id` BCSQ check is correct for
|
|
the intended OBP model where OBP-to-OBP TG rewrite does not occur.
|
|
- Treat the deterministic OBP-to-OBP rewrite probe as an invalid scenario unless
|
|
bridge validation later decides to explicitly support or reject such entries.
|
|
|
|
## Dial-A-TG Product Rationale
|
|
|
|
### Findings
|
|
- Dial-a-TG exists to let a terminal user access any TG on the FreeDMR network
|
|
without explicitly programming that TG into the terminal/codeplug.
|
|
- Codeplug generation and DMR terminal programming can be a major barrier for
|
|
amateur radio users. Many users are strong RF/electronics engineers but may
|
|
not be interested in, or comfortable with, computer-based configuration.
|
|
- FreeDMR's amateur-radio use case differs from DMR's intended commercial use
|
|
case, so some FreeDMR behavior intentionally optimizes usability and network
|
|
access rather than strict commercial fleet/radio programming assumptions.
|
|
|
|
### Assumptions
|
|
- Dial-a-TG changes should be evaluated against this access/usability goal, not
|
|
only against narrow protocol or commercial-radio assumptions.
|
|
|
|
### Protocol-Sensitive Areas
|
|
- Dial-a-TG intentionally maps local RF control/use of TG9 TS2 to wider-network
|
|
TG access on FBP/OBP.
|
|
- This makes TG namespace clarity critical: RF/client-visible TGs and
|
|
OBP/FBP-visible TGs can intentionally differ.
|
|
|
|
### Inferred Invariants
|
|
- Dial-a-TG should reduce terminal programming burden for users.
|
|
- Maintaining usable audio/network access for amateur users is a first-class
|
|
design constraint.
|
|
|
|
## FreeDMR Routing Model
|
|
|
|
### Findings
|
|
- FreeDMR can be understood like a PBX:
|
|
- TGs are conference groups that can be connected to.
|
|
- DMR IDs are like phone numbers.
|
|
- Routing is centered on TGs and DMR IDs.
|
|
- FreeDMR is intended to be timeslot agnostic, unlike some systems.
|
|
- Timeslots are closer to phone lines:
|
|
- a simplex hotspot has one usable line;
|
|
- a repeater or duplex hotspot has two usable lines.
|
|
|
|
### Assumptions
|
|
- Routing changes should preserve TG/DMR-ID centric behavior.
|
|
- Timeslot should be treated as an available access path or capacity dimension,
|
|
not as the primary identity of a route unless a specific feature explicitly
|
|
requires slot scoping.
|
|
|
|
### Protocol-Sensitive Areas
|
|
- Dial-a-TG control from TS1 affecting TS2 reflector state is consistent with
|
|
the PBX/line model: the user can use one line to control which conference is
|
|
connected on another line.
|
|
- Tests should avoid assuming commercial DMR fleet semantics where timeslot is
|
|
always part of the primary routing identity.
|
|
|
|
### Inferred Invariants
|
|
- TG and DMR ID are the primary routing identities.
|
|
- Timeslot handling should preserve capacity/access behavior without making
|
|
FreeDMR unnecessarily slot-bound.
|
|
|
|
## HBP-Side Tolerance Principle
|
|
|
|
### Findings
|
|
- FreeDMR should generally be more permissive on the HBP side because HBP
|
|
represents direct RF-facing connections to repeaters, duplex hotspots and
|
|
simplex hotspots.
|
|
- HBP-connected devices may come from several vendors and may run proprietary or
|
|
open source implementations with differing interpretations of the protocol.
|
|
- RF-facing behavior may vary in timing, late-entry recovery, stream continuity,
|
|
data handling and terminator delivery.
|
|
|
|
### Assumptions
|
|
- HBP-side handling should avoid over-strict enforcement unless needed for
|
|
safety, loop prevention, routing correctness or preventing network harm.
|
|
- OBP/FBP peer-server handling can be stricter because it is server-to-server and
|
|
less directly exposed to varied RF terminal/repeater behavior.
|
|
|
|
### Protocol-Sensitive Areas
|
|
- Timeout behavior on HBP should remain tolerant of resumed streams after packet
|
|
gaps unless live testing proves this causes worse failures.
|
|
- Data packet handling may need extra tolerance because terminal implementations
|
|
can be loose or incomplete.
|
|
|
|
### Inferred Invariants
|
|
- Prefer audio continuity and interoperability at the RF-facing HBP boundary.
|
|
- Treat strictness on HBP as a deliberate choice requiring evidence.
|
|
|
|
## Unit-Unit Private Call Policy
|
|
|
|
### Findings
|
|
- FreeDMR deliberately does not allow user-to-user unit-unit private calls.
|
|
- Amateur Radio is an open service, not a private communications system.
|
|
- A unit-unit private call still occupies an RF timeslot even when other
|
|
stations cannot hear useful traffic on that slot.
|
|
- Users who want directed or pseudo-private routing should use a TG instead,
|
|
including the group-call TG that corresponds to their DMR ID if appropriate.
|
|
- Unit-unit/private calls are reserved for control purposes, including
|
|
dial-a-TG and related control flows.
|
|
|
|
### Assumptions
|
|
- Future changes should not add general private voice routing unless this policy
|
|
is explicitly changed.
|
|
- Unit-call handling should continue to distinguish control-plane private calls
|
|
from user traffic.
|
|
|
|
### Protocol-Sensitive Areas
|
|
- DMR allows unit-unit calls, but FreeDMR policy intentionally restricts them in
|
|
favor of open group-call behavior.
|
|
- Test cases involving private calls should treat them as control operations,
|
|
not as a supported user voice routing path.
|
|
|
|
### Inferred Invariants
|
|
- User voice traffic should be routed as group calls/TGs.
|
|
- Private calls are control-plane only in FreeDMR.
|
|
|
|
## HBP New Voice Terminator Can Use Stale Data Classification
|
|
|
|
### Findings
|
|
- `bridge_master.py`, `routerHBP.dmrd_received()`, computes `_data_control` for
|
|
each HBP group/vcsbk packet.
|
|
- The slot's `RX_DATA_STREAM` / `RX_GROUP_VOICE_STREAM` classification is only
|
|
updated at the end of the group branch, after final terminator handling.
|
|
- Final terminator handling checks `self.STATUS[_slot].get('RX_DATA_STREAM')` to
|
|
decide whether to emit `GROUP VOICE,END,RX` and mark
|
|
`RX_FINISHED_STREAM_ID`.
|
|
- Deterministic probe:
|
|
- Inject group data header on MASTER-A TS2, leaving `RX_DATA_STREAM=True`.
|
|
- Inject a new voice stream where the first observed packet is a voice
|
|
terminator.
|
|
- The new stream emits `GROUP VOICE,START,RX`, but the terminator final-action
|
|
path sees stale `RX_DATA_STREAM=True`, so it does not emit voice END and does
|
|
not mark `RX_FINISHED_STREAM_ID`.
|
|
- A later same-stream voice burst is then routed, increasing the target packet
|
|
count from 2 to 3.
|
|
|
|
### Assumptions To Validate
|
|
- A newly observed HBP stream should set its data-vs-voice classification before
|
|
any final-action handling for that same packet.
|
|
- If FreeDMR first observes a stream at its terminator, the classification should
|
|
come from that terminator packet, not from the previous slot occupant.
|
|
- Even if this is an edge case, stale data classification should not suppress
|
|
voice lifecycle/finished-stream handling for a new voice stream.
|
|
|
|
### Unresolved Questions
|
|
- How often real RF/HBP paths deliver a new stream whose first observed packet is
|
|
a terminator. It can plausibly occur around packet loss, startup, or missed
|
|
earlier packets, but may be rare.
|
|
- Whether strict handling should suppress a terminator-only "start" report. This
|
|
proposal does not change that existing behavior; it only makes the end/finish
|
|
classification match the current packet.
|
|
|
|
### Protocol-Sensitive Areas
|
|
- This is HBP source slot lifecycle state, not packet mutation.
|
|
- DMR terminal behavior may be loose; preserving robust audio/lifecycle behavior
|
|
matters more than assuming perfectly ordered full streams.
|
|
|
|
### Inferred Invariants
|
|
- Per-slot packet classification must describe the current stream before the
|
|
current packet's lifecycle side effects are applied.
|
|
- Stale data-control state must not leak into a new voice stream.
|
|
|
|
### Resolution
|
|
- HBP group/vcsbk new-stream handling now sets
|
|
`RX_DATA_STREAM = _data_control` and
|
|
`RX_GROUP_VOICE_STREAM = not _data_control` before terminator final actions can
|
|
run.
|
|
- Kept the existing end-of-branch assignment for ongoing packets.
|
|
- Added deterministic coverage proving a voice terminator observed after a data
|
|
stream marks the new voice stream finished and suppresses later same-stream
|
|
bursts.
|
|
|
|
## HBP Terminator-Only Voice On Idle Slot Skips Finished Handling
|
|
|
|
### Findings
|
|
- `bridge_master.py`, `routerHBP.dmrd_received()`, final voice terminator
|
|
handling is guarded by:
|
|
`(_frame_type == HBPF_DATA_SYNC) and (_dtype_vseq == HBPF_SLT_VTERM) and
|
|
(self.STATUS[_slot]['RX_TYPE'] != HBPF_SLT_VTERM)`.
|
|
- If the first observed packet for a new stream is a voice terminator while the
|
|
slot was idle, the previous slot `RX_TYPE` is already `HBPF_SLT_VTERM`, so
|
|
final terminator handling is skipped.
|
|
- Deterministic probe:
|
|
- Start from idle MASTER-A TS2.
|
|
- Inject a new HBP group voice terminator for stream `01020304`.
|
|
- The packet is forwarded and `GROUP VOICE,START,RX` is emitted, but no
|
|
`GROUP VOICE,END,RX` is emitted and `RX_FINISHED_STREAM_ID` remains null.
|
|
- A later same-stream voice burst is then forwarded, increasing target capture
|
|
count from 1 to 2 and setting `RX_TYPE` to the late voice vseq.
|
|
|
|
### Assumptions To Validate
|
|
- If FreeDMR accepts and forwards a terminator-only voice packet as a new voice
|
|
stream, it should also close that stream locally and suppress later same-stream
|
|
voice.
|
|
- The final-action decision should be based on whether the current packet is a
|
|
voice terminator for the current stream, not only on the previous slot
|
|
`RX_TYPE`.
|
|
- This should stay limited to non-data voice streams.
|
|
|
|
### Unresolved Questions
|
|
- Whether a terminator-only packet should emit both START and END reports, or
|
|
whether START should be suppressed when no earlier voice packet was observed.
|
|
Existing behavior already emits START, so the minimal fix should only add the
|
|
missing end/finished handling.
|
|
|
|
### Protocol-Sensitive Areas
|
|
- This is HBP source lifecycle/late-packet suppression, not packet mutation.
|
|
- A terminator-only first observation can happen when earlier packets were lost
|
|
or FreeDMR starts observing mid-stream.
|
|
|
|
### Inferred Invariants
|
|
- A voice terminator should close the current stream even if the previous slot
|
|
state was idle.
|
|
- Post-terminator same-stream voice should not be routed.
|
|
|
|
### Resolution
|
|
- Replaced the final terminator guard's previous-`RX_TYPE` dependency with a
|
|
current voice-terminator condition that avoids duplicate final handling for an
|
|
already-finished stream.
|
|
- Added deterministic coverage for an idle-slot terminator-only HBP voice packet
|
|
marking the stream finished and suppressing later same-stream voice.
|
|
|
|
## HBP Timeout Cleanup Does Not Mark Stream Finished
|
|
|
|
### Findings
|
|
- `bridge_master.py`, `stream_trimmer_loop()`, HBP RX timeout cleanup sets
|
|
`_slot['RX_TYPE'] = HBPF_SLT_VTERM` and emits `GROUP VOICE,END,RX` for
|
|
reportable voice streams.
|
|
- It does not set `RX_FINISHED_STREAM_ID`.
|
|
- Because `RX_STREAM_ID` remains the timed-out stream ID until the 60-second
|
|
null cleanup, a later same-stream voice packet before that cleanup is treated
|
|
as a continuation rather than a new/finished stream.
|
|
- Deterministic probe:
|
|
- Inject HBP group voice stream `01020304`.
|
|
- Advance six seconds and run `stream_trimmer_loop()`.
|
|
- Timeout emits `GROUP VOICE,END,RX`, leaves `RX_STREAM_ID=01020304`, but
|
|
`RX_FINISHED_STREAM_ID` remains null.
|
|
- A later same-stream voice burst routes to the target, increasing target
|
|
capture count from 1 to 2 and setting `RX_TYPE` to the late voice vseq.
|
|
|
|
### Updated Analysis
|
|
- User clarified that timeout can represent two different failure modes:
|
|
- RF path loss between terminal and base station, where the terminal's
|
|
terminator may also be lost.
|
|
- Network/server/mesh packet path disruption, where an otherwise valid stream
|
|
may have a long packet gap and later resume with the same stream ID.
|
|
- In RF-path loss, if the user is still transmitting and comes back into range,
|
|
the base station may recover via DMR late entry and may or may not assign a new
|
|
HBP stream ID. This needs live RF testing.
|
|
- In network-path loss, downstream servers cannot reliably distinguish a truly
|
|
ended over from a temporarily missing packet flow. A later same-stream packet
|
|
may be legitimate and should probably be forwarded to preserve audio.
|
|
|
|
### Revised Assumptions
|
|
- Explicit terminators and timeout cleanup should not be treated identically.
|
|
- A real terminator is a strong end-of-stream signal and should mark the stream
|
|
finished.
|
|
- A timeout is an observability/liveness fallback for missing terminators, but
|
|
may be a soft end when the underlying stream could resume after network delay.
|
|
- FreeDMR should prioritize preserving recovered audio over rigidly enforcing a
|
|
timeout as a hard stream close when no terminator was seen.
|
|
|
|
### Unresolved Questions
|
|
- Live RF testing should determine whether an RF-lost/recovered over uses a new
|
|
stream ID or resumes the old one.
|
|
- Black-box/network simulation should test whether a packet gap longer than the
|
|
HBP timeout can occur in the mesh and then resume with the same stream ID.
|
|
- Reporting may remain imperfect in this case: a timeout `END,RX` can be emitted
|
|
and later packets for the same stream may still be forwarded without a fresh
|
|
`START,RX`.
|
|
|
|
### Protocol-Sensitive Areas
|
|
- This is HBP source stream lifecycle/late-packet suppression, not packet
|
|
mutation.
|
|
- It directly follows the operational purpose of timeout cleanup as a substitute
|
|
terminator when the real terminator was lost.
|
|
|
|
### Inferred Invariants
|
|
- An explicit terminator closes a stream; a timeout without terminator is less
|
|
certain and may need to remain recoverable.
|
|
- Audio continuity after network gaps is more important than making timeout
|
|
reports look perfectly final.
|
|
- Timeout reports should be understood as "stream appears lost" rather than
|
|
definitive proof that the over ended.
|
|
|
|
### Resolution
|
|
- No runtime change for now.
|
|
- Do not mark HBP streams finished on timeout until live RF and network-gap
|
|
testing proves this will not block legitimate recovered audio.
|
|
- Keep the existing behavior documented as a deliberate soft-timeout tradeoff:
|
|
reporting may show an END for a lost stream, but later same-stream packets can
|
|
still route before the 60-second null cleanup.
|
|
|
|
### Follow-Up Verification
|
|
- Rechecked earlier lifecycle fixes against this soft-timeout model.
|
|
- HBP timeout cleanup still only sets `RX_TYPE = HBPF_SLT_VTERM`, clears
|
|
data/report classification, and optionally emits the timeout report. It does
|
|
not set `RX_FINISHED_STREAM_ID`.
|
|
- Deterministic probe confirmed HBP timeout remains recoverable:
|
|
- one packet routed before timeout;
|
|
- timeout report emitted with `RX_FINISHED_STREAM_ID == 0`;
|
|
- later same-stream packet before 60-second null cleanup still routes.
|
|
- Explicit terminators remain hard end-of-stream signals:
|
|
- HBP explicit terminator sets `RX_FINISHED_STREAM_ID` and suppresses later
|
|
same-stream voice;
|
|
- forwarded HBP -> OBP / OBP -> OBP terminators set target `_fin`;
|
|
- OBP source timeout sets `_to`, not `_fin`.
|
|
- Conclusion: earlier fixes did not convert timeout cleanup into a hard finished
|
|
state. The hard-close behavior is currently limited to real/forwarded
|
|
terminators.
|
|
|
|
## DMRD Sequence Wrap In Voice Duplicate Control
|
|
|
|
### Findings
|
|
- `hblink.py` parses the DMRD sequence number as `_seq = _data[4]`, so it is a
|
|
one-byte value in the range `0..255`.
|
|
- `bridge_master.py`, `routerHBP.dmrd_received()`, HBP group/vcsbk duplicate
|
|
control treats sequence numbers as a simple increasing integer:
|
|
duplicate when `_seq == lastSeq`, out-of-order when
|
|
`_seq < lastSeq and _seq != 1`, and missed packets when
|
|
`_seq > lastSeq + 1`.
|
|
- `bridge_master.py`, `routerOBP.dmrd_received()`, OBP group/vcsbk duplicate
|
|
control uses the same linear comparisons.
|
|
- This works until sequence wrap. If the last accepted sequence is `255` and
|
|
post-wrap packets `0` and `1` are lost, a valid resumed packet with sequence
|
|
`2` is treated as out-of-order because `2 < 255 and 2 != 1`.
|
|
- Because the discard path does not advance `lastSeq`, later valid packets `3`,
|
|
`4`, and so on remain less than `255` and can continue to be discarded.
|
|
|
|
### Assumptions
|
|
- DMRD sequence numbers are modulo-256 transport sequencing, not an unbounded
|
|
stream counter.
|
|
- Packet `0` is a valid DMRD sequence value, not "no sequence"; the current
|
|
truthiness checks skip some validation when `_seq == 0`.
|
|
- Duplicate and out-of-order suppression is intended to drop repeated or stale
|
|
packets, not to mute a stream after a legitimate sequence wrap with packet
|
|
loss.
|
|
|
|
### Unresolved Questions
|
|
- Whether data/control paths should use the same helper if future duplicate
|
|
control is added there, while remembering that DMR data packets are
|
|
packet-oriented rather than continuous voice streams.
|
|
|
|
### Protocol-Sensitive Areas
|
|
- This affects packet-control behavior for long voice streams and network gaps,
|
|
especially near sequence wrap.
|
|
- HBP should remain tolerant of RF-facing packet loss and vendor differences.
|
|
- OBP/FBP should also handle modulo sequence wrap because the sequence value is
|
|
carried in DMRD packet format, even though server-to-server behavior can be
|
|
stricter in other areas.
|
|
|
|
### Inferred Invariants
|
|
- DMRD sequence comparison should be modulo-aware.
|
|
- A post-wrap packet should be treated as forward progress even if one or more
|
|
immediately post-wrap packets were lost.
|
|
- Duplicate detection should include sequence `0`.
|
|
|
|
### Resolution
|
|
- Added `dmrd_seq_delta(seq, last_seq)` to calculate sequence progress as
|
|
`(seq - last_seq) % 256`.
|
|
- Updated HBP and OBP group/vcsbk packet-control paths to classify delta `0` as
|
|
duplicate, delta `1` as normal progress, delta `2..127` as forward progress
|
|
with missed packets, and delta `128..255` as old/out-of-order.
|
|
- Removed the `_seq > 0` guard from hash duplicate checks so sequence `0` cannot
|
|
bypass duplicate detection.
|
|
- Added deterministic tests for HBP and OBP voice streams crossing `254`, `255`,
|
|
then `2`, and for HBP sequence `0` duplicate suppression.
|
|
|
|
## HBP/OBP Source Timeout Uses Stream Start Time
|
|
|
|
### Findings
|
|
- `bridge_master.py`, `routerHBP.dmrd_received()`, HBP group/vcsbk handling has
|
|
a source timeout check:
|
|
`if self.STATUS[_slot]['RX_START'] + 180 < pkt_time`.
|
|
- `bridge_master.py`, `routerOBP.dmrd_received()`, OBP group/vcsbk handling has
|
|
the analogous check:
|
|
`if self.STATUS[_stream_id]['START'] + 180 < pkt_time`.
|
|
- Both branches log that the source/stream should be ignored and then return
|
|
before duplicate control, routing, terminator handling, and normal state
|
|
updates.
|
|
- Because the comparison is against the stream start time, this is a hard
|
|
maximum stream duration of 180 seconds. It is not an inactivity timeout.
|
|
- A still-active voice stream that continues past 180 seconds without a
|
|
terminator will be dropped even if packets continue arriving at normal
|
|
cadence.
|
|
|
|
### Assumptions
|
|
- User confirmed this is deliberate amateur DMR network behavior.
|
|
- The 180-second guard is a hard maximum over/stream duration, comparable to a
|
|
network-side time-out timer.
|
|
- Terminal time-out timers are also advised to be set to 180 seconds.
|
|
|
|
### Unresolved Questions
|
|
- None for now.
|
|
|
|
### Protocol-Sensitive Areas
|
|
- This is voice stream lifecycle and loop/source protection, not packet mutation.
|
|
- HBP paths are RF-facing and may reflect repeater/hotspot time-out timer
|
|
behavior, so changing this could permit overlong RF streams that deployments
|
|
currently expect to be cut off.
|
|
- OBP/FBP can carry arbitrary forwarded streams, so the operational impact may
|
|
differ between HBP and OBP.
|
|
|
|
### Inferred Invariants
|
|
- A hard stream-age cap should be explicit and documented if retained.
|
|
- The current 180-second source timeout should remain based on original stream
|
|
start time, not last packet activity.
|
|
|
|
### Resolution
|
|
- No runtime change. The `START + 180` behavior is intentional and should not be
|
|
rewritten as a last-activity timeout.
|
|
|
|
## Generated Prompt Cancellation Does Not Send Terminator
|
|
|
|
### Findings
|
|
- `mk_voice.py`, `pkt_gen()`, builds generated voice prompts as a DMRD stream:
|
|
three voice headers, AMBE bursts, and a final voice terminator frame using the
|
|
same generated stream ID.
|
|
- `bridge_master.py`, `sendSpeech()`, `disconnectedVoice()`, and
|
|
`playFileOnRequest()` now stop prompt scheduling when
|
|
`_generatedVoiceCancelled()` becomes true.
|
|
- Those loops break immediately and do not drain the generator to its final
|
|
terminator packet.
|
|
- `_cancelGeneratedVoice()` only marks prompt state as cancelled; it does not
|
|
send a same-stream terminator for the abandoned prompt.
|
|
- A real HBP voice stream can therefore interrupt a generated prompt on an HBP
|
|
target slot after the repeater has received a prompt header/burst but before it
|
|
receives the generated prompt terminator.
|
|
|
|
### Assumptions
|
|
- User confirmed that absence of a terminator for an interrupted prompt is not a
|
|
major terminal-facing issue.
|
|
- DMR is designed for lossy environments, and a missing terminator is an
|
|
expected recoverable condition.
|
|
- DMR also supports interruption concepts such as priority interruption, so
|
|
receivers should tolerate an interrupted stream being superseded.
|
|
- Any future fix must not delay the real RF/network voice that caused the
|
|
cancellation.
|
|
|
|
### Unresolved Questions
|
|
- None for now.
|
|
|
|
### Protocol-Sensitive Areas
|
|
- This is generated local HBP TX lifecycle, not inbound DMR packet parsing.
|
|
- The fix would intentionally create or forward a terminator packet for locally
|
|
generated voice; it must preserve the stream ID and LC namespace expected by
|
|
the target repeater/client.
|
|
- Prompt interruption exists to prioritize live RF/network audio, so any
|
|
terminator fix must be low latency.
|
|
|
|
### Inferred Invariants
|
|
- Real RF/network audio should remain higher priority than locally generated
|
|
prompts.
|
|
- Prompt termination should not mutate the real stream that caused the
|
|
interruption.
|
|
- Missing generated-prompt terminators are acceptable if interruption preserves
|
|
live traffic.
|
|
|
|
### Resolution
|
|
- No runtime change. Prompt cancellation may abandon the generated prompt without
|
|
sending its final terminator.
|
|
|
|
## Voice Ident Cancellation Leaves Prompt Cancel State Set
|
|
|
|
### Findings
|
|
- `bridge_master.py`, `sendSpeech()`, `disconnectedVoice()`, and
|
|
`playFileOnRequest()` wrap generated prompt playback with
|
|
`_beginGeneratedVoice()` and `_endGeneratedVoice()`.
|
|
- `bridge_master.py`, `ident()` sends generated packets directly in its own loop
|
|
and calls `sendVoicePacket()` without `_beginGeneratedVoice()` /
|
|
`_endGeneratedVoice()` or `_generatedVoiceCancelled()` checks.
|
|
- `sendVoicePacket()` returns immediately if `_slot['TX_PROMPT_CANCEL']` is true.
|
|
- Real HBP voice can cancel generated prompt state on a target slot by calling
|
|
`_cancelGeneratedVoice()`, setting `TX_PROMPT_CANCEL=True` and
|
|
`TX_PROMPT_ACTIVE=False`.
|
|
- If that happens while a voice ident is active, the ident loop keeps scheduling
|
|
packets that `sendVoicePacket()` drops, and no ident cleanup resets
|
|
`TX_PROMPT_CANCEL`.
|
|
- A later `ident()` run on the same slot can then have all of its packets dropped
|
|
immediately because it never begins a new prompt token or clears the old cancel
|
|
flag.
|
|
|
|
### Assumptions
|
|
- Voice ident should follow the same generated-prompt lifecycle as dial-a-TG
|
|
prompts and on-demand files.
|
|
- Real RF/network voice should still be able to interrupt voice ident.
|
|
- A cancelled ident should not permanently suppress later idents.
|
|
|
|
### Unresolved Questions
|
|
- None for now.
|
|
|
|
### Protocol-Sensitive Areas
|
|
- This is generated local HBP TX scheduling/state, not inbound DMR parsing.
|
|
- The fix should not alter generated packet bytes, destination selection, or the
|
|
existing policy that ident only runs when the slot has been idle.
|
|
|
|
### Inferred Invariants
|
|
- Every generated voice loop that uses `sendVoicePacket()` should own a prompt
|
|
token and clear/finish prompt state when it exits.
|
|
- Interrupting one generated voice ident must not permanently block future
|
|
generated voice idents.
|
|
|
|
### Resolution
|
|
- Updated `ident()` to use `_beginGeneratedVoice()`,
|
|
`_generatedVoiceCancelled()`, and `_endGeneratedVoice()` around its generated
|
|
packet loop.
|
|
- Removed unused `_stream_id` / `_pkt_time` local assignments from the ident
|
|
loop.
|
|
- Added deterministic coverage proving a cancelled ident does not leave the slot
|
|
in a state that blocks a later ident.
|
|
|
|
## HBP New Voice Stream Keeps Previous Duplicate State
|
|
|
|
### Findings
|
|
- `bridge_master.py`, `routerHBP.dmrd_received()`, HBP group/vcsbk handling uses
|
|
per-slot duplicate-control state: `lastSeq`, `lastData`, `loss`, `packets`,
|
|
and `crcs`.
|
|
- When a new stream is detected with
|
|
`_stream_id != self.STATUS[_slot]['RX_STREAM_ID']`, the code resets
|
|
`packets`, `loss`, and `crcs`.
|
|
- It does not reset `lastSeq` or `lastData` at that point.
|
|
- The duplicate/out-of-order block for the current packet then runs against
|
|
potentially stale `lastSeq` and `lastData` from the previous stream on the
|
|
same slot.
|
|
- If the previous stream ended by explicit terminator, final terminator handling
|
|
resets `lastSeq` and `lastData`, so the issue is masked.
|
|
- If the previous stream ended by timeout, source timeout, collision gap, or any
|
|
path that does not run explicit terminator cleanup, the next stream can be
|
|
judged against stale sequence/data.
|
|
- Example from code logic: previous stream leaves `lastSeq=200`; a new stream
|
|
begins with sequence `1`. `dmrd_seq_delta(1, 200)` is `57`, so the packet is
|
|
treated as forward progress with missed packets on the old stream rather than
|
|
the first packet of a new stream. Other stale values can classify the new
|
|
packet as duplicate or out-of-order.
|
|
|
|
### Assumptions
|
|
- Duplicate-control state is stream-scoped, not slot-lifetime-scoped.
|
|
- A new stream on the same HBP slot should begin with `lastSeq=False` and
|
|
`lastData=False`, just as OBP initializes per-stream state.
|
|
- Resetting duplicate-control state on new stream should not affect explicit
|
|
terminator suppression, which is now handled separately by
|
|
`RX_FINISHED_STREAM_ID`.
|
|
|
|
### Unresolved Questions
|
|
- None for now.
|
|
|
|
### Protocol-Sensitive Areas
|
|
- This is HBP RF-facing packet-control state, not packet mutation.
|
|
- HBP should be tolerant of missing terminators and stream transitions after
|
|
packet loss.
|
|
- The fix should not change OBP, where stream state is already per stream.
|
|
|
|
### Inferred Invariants
|
|
- Duplicate and sequence state must belong to the current stream.
|
|
- Missing a terminator must not let stale duplicate-control state from the old
|
|
stream suppress or misclassify the next HBP stream.
|
|
|
|
### Resolution
|
|
- Updated the HBP group/vcsbk new-stream initialization block to reset
|
|
`lastSeq` and `lastData` along with `packets`, `loss`, and `crcs`.
|
|
- Added deterministic coverage for a prior HBP stream that times out with
|
|
`lastSeq=200`; the next stream on the same slot starts with sequence `1`,
|
|
routes to the target, and records no inherited loss.
|
|
|
|
## HBP Sequence Gaps on Unreliable Networks
|
|
|
|
### Findings
|
|
- The DMRD sequence byte is supplied by the HBP client/repeater/hotspot at
|
|
packet offset 4 and is passed through `hblink.py` into
|
|
`routerHBP.dmrd_received()`.
|
|
- The current modulo-256 check treats deltas 1..127 as forward progress,
|
|
delta 0 as duplicate, and deltas greater than 127 as stale/out-of-order.
|
|
- If a network-side gap exceeds half the 8-bit sequence space, the server
|
|
cannot distinguish "very late old packet" from "forward packet after a long
|
|
loss".
|
|
- Because the out-of-order branch returns before updating `lastSeq`, a
|
|
same-stream resume after a greater-than-127 jump may remain muted until the
|
|
sequence wraps back near the previous accepted value.
|
|
|
|
### Assumptions
|
|
- RF-side loss between the terminal and repeater may not produce a sequence jump
|
|
at the server if the HBP device only increments the DMRD sequence for packets
|
|
it actually sends.
|
|
- Network-side loss between an HBP device and the server can produce large
|
|
sequence jumps because packets were sent but not received by FreeDMR.
|
|
- HBP should remain permissive where practical because it represents direct RF
|
|
paths and unreliable access networks.
|
|
|
|
### Unresolved Questions
|
|
- Whether the HBP duplicate-control path should eventually add a long-gap
|
|
recovery rule for same stream IDs after a quiet interval, rather than treating
|
|
all greater-than-127 deltas as stale.
|
|
|
|
### Protocol-Sensitive Areas
|
|
- The sequence byte is only 8 bits, so long-gap direction is inherently
|
|
ambiguous without timing, stream lifecycle, or additional protocol context.
|
|
- Any recovery rule must avoid accepting genuinely old/reordered packets and
|
|
creating audio or loop-control regressions.
|
|
|
|
### Inferred Invariants
|
|
- Small packet loss and sequence wrap must not suppress a voice stream.
|
|
- A new HBP stream must not inherit sequence state from an old stream.
|
|
- Very long same-stream gaps require a policy decision: prefer stale-packet
|
|
rejection or prefer late audio recovery on lossy access networks.
|
|
|
|
## FBP Sequence Gaps on Unreliable Links
|
|
|
|
### Findings
|
|
- OBP/FBP group voice handling uses the same modulo-256 sequence delta policy
|
|
as HBP: delta 0 is duplicate, deltas 1..127 are forward progress, and deltas
|
|
greater than 127 are treated as stale/out-of-order.
|
|
- OBP/FBP duplicate-control state is keyed by stream ID, so it does not have
|
|
the HBP per-slot stale-new-stream problem.
|
|
- A same-stream long network outage on an OBP/FBP link can still produce the
|
|
same recovery issue: the first packets after the outage may be rejected until
|
|
the sequence wraps back near the last accepted value.
|
|
- OBP/FBP code updates `LAST` on accepted packets and on some loop/timeout
|
|
returns. For the duplicate/out-of-order branch, the useful quiet-time signal
|
|
is the elapsed time since the last accepted/routed packet.
|
|
|
|
### Assumptions
|
|
- Some FreeDMR peer-server links may be unreliable because of RF IP, cellular,
|
|
portable, EMCOMM, or less-developed infrastructure use cases.
|
|
- Server-to-server links still need stricter loop-control behavior than HBP
|
|
edges, but rejecting live resumed audio after a long quiet gap is undesirable.
|
|
- Any FBP recovery rule should apply only to stream media packets after loop
|
|
and source-selection checks have already accepted this server as the first
|
|
source for the stream.
|
|
|
|
### Unresolved Questions
|
|
- Whether HBP and FBP should share the same long-gap threshold, or FBP should
|
|
use a slightly higher threshold to reduce risk from delayed/reordered mesh
|
|
packets.
|
|
|
|
### Protocol-Sensitive Areas
|
|
- BCSQ/source-quench correctness depends on stream ID and TG; recovery should
|
|
not bypass existing first-source/loop-control decisions.
|
|
- OBP/FBP can carry multiple concurrent streams, so rate and recovery decisions
|
|
must remain per stream, not global.
|
|
|
|
### Inferred Invariants
|
|
- FBP should not discard live recovered audio solely because a lossy link
|
|
crossed the half-sequence ambiguity point.
|
|
- FBP long-gap recovery must not weaken source-selection, STUN, ACL, TG filter,
|
|
or source-quench behavior.
|
|
|
|
## FBP Trunk-Wide Long-Gap Diagnostics
|
|
|
|
### Findings
|
|
- OBP/FBP links can carry many independent streams over one peer connection.
|
|
- If long-gap recovery is observed on only some streams, the loss may have been
|
|
inherited from an upstream route taken by those streams.
|
|
- If long-gap recovery is observed across most or all active streams on the
|
|
same OBP/FBP peer connection within a short window, that suggests the local
|
|
upstream link or peer connection is struggling.
|
|
|
|
### Assumptions
|
|
- This signal is diagnostic only and must not influence packet admission,
|
|
source selection, loop control, or source-quench decisions.
|
|
- Per-stream recovery logging is useful for debugging a stream; trunk-wide
|
|
aggregate logging is useful for diagnosing infrastructure or upstream link
|
|
trouble.
|
|
|
|
### Unresolved Questions
|
|
- What threshold should define "most/all streams" on a trunk: all active
|
|
streams, a fixed minimum count, or a ratio such as 80% within a short window.
|
|
- Whether this should be implemented immediately with long-gap recovery or left
|
|
as a later observability improvement.
|
|
|
|
### Protocol-Sensitive Areas
|
|
- A malicious or delayed packet pattern must not be able to relax loop-control
|
|
or routing checks by triggering trunk-wide diagnostics.
|
|
- Trunk-wide diagnostics must be rate-limited to avoid log flooding during real
|
|
network incidents.
|
|
|
|
### Inferred Invariants
|
|
- Long-gap recovery remains per stream.
|
|
- Trunk-wide detection is warning-only observability.
|
|
- Diagnostics must never bypass STUN, HMAC/authentication, ACLs, TG filters,
|
|
loop-control, finished-stream suppression, source timeout, rate limiting, or
|
|
BCSQ/source-quench behavior.
|
|
|
|
## OBP Target LC Missing-Key Logging Uses Wrong System Variable
|
|
|
|
### Findings
|
|
- `bridge_master.py`, `routerOBP.to_target()`, OpenBridge-target voice rewrite
|
|
branch catches missing `T_LC` and `EMB_LC` state with `except KeyError`.
|
|
- Both handlers log with `system` instead of `self._system`.
|
|
- `system` is not a local variable in `to_target()`. If no module-level
|
|
`system` binding exists, the exception handler itself can raise `NameError`.
|
|
If a module-level binding does exist from other loops, the log can report the
|
|
wrong system.
|
|
- The surrounding code intends these `KeyError` paths to be non-fatal: one logs
|
|
and continues processing the terminator, the other logs and skips the packet.
|
|
|
|
### Assumptions
|
|
- Missing LC state is abnormal, but the existing handlers show the intended
|
|
behavior is to avoid crashing the router on this condition.
|
|
- Changing the log argument from `system` to `self._system` does not alter packet
|
|
routing, mutation, or normal successful voice handling.
|
|
|
|
### Unresolved Questions
|
|
- Whether the terminator missing-`T_LC` path should also skip sending the packet
|
|
after logging. That is a separate behavior question and should not be bundled
|
|
with the logging fix.
|
|
|
|
### Protocol-Sensitive Areas
|
|
- This is an error-path logging bug in OBP target voice LC rewrite handling.
|
|
- It should not change LC rewrite behavior when `T_LC` / `EMB_LC` are present.
|
|
|
|
### Inferred Invariants
|
|
- Error-path logging must not introduce a new exception while handling malformed
|
|
or inconsistent stream state.
|
|
- Logs should identify the router instance handling the packet.
|
|
|
|
### Resolution
|
|
- Replaced the non-local `system` log argument with `self._system` in the
|
|
missing `T_LC` and `EMB_LC` handlers.
|
|
- Added deterministic OBP-to-OBP coverage that removes target `EMB_LC` state,
|
|
injects a voice burst, and verifies the warning logs without crashing or
|
|
forwarding the malformed rewrite.
|
|
|
|
## Widened Review: Startup, Config, Support Functions
|
|
|
|
### Findings
|
|
- `config.py`, `build_config()`, reads `GLOBAL.USE_ACL` with
|
|
`config.get(..., fallback=True)` instead of `config.getboolean(...)`.
|
|
Therefore a config value such as `USE_ACL: False` becomes the non-empty string
|
|
`"False"`, which is truthy in packet ACL checks.
|
|
- Packet ACL checks are split across both layers: `hblink.py` performs low-level
|
|
admission checks using `self._CONFIG['GLOBAL']['USE_ACL']`, and
|
|
`bridge_master.py` checks the same global flag before target forwarding. A
|
|
truthy string therefore affects both layers consistently, but incorrectly.
|
|
- `config.py` converts `ALIASES.STALE_DAYS` into seconds as `STALE_TIME`.
|
|
`bridge_master.py` then schedules alias reloads with
|
|
`CONFIG['ALIASES']['STALE_TIME'] * 86400`, multiplying by 86400 twice.
|
|
A one-day alias stale interval therefore schedules periodic reload around
|
|
86400 days instead of 1 day.
|
|
- `bridge_master.py`, `setAlias()`, assigns reloaded alias dictionaries to local
|
|
variables named `peer_ids`, `subscriber_ids`, `talkgroup_ids`,
|
|
`local_subscriber_ids`, `server_ids`, and `checksums`. Without a `global`
|
|
declaration or updating `CONFIG`, the periodic alias reload does not update
|
|
the module-level alias dictionaries used by logging, reports, and routing
|
|
helpers.
|
|
- `hblink.py` router classes also read aliases from the shared config object:
|
|
`CONFIG['_SUB_IDS']`, `CONFIG['_PEER_IDS']`,
|
|
`CONFIG['_LOCAL_SUBSCRIBER_IDS']`, and `CONFIG['_SERVER_IDS']`. Therefore an
|
|
alias reload fix must update both `bridge_master.py` globals and these shared
|
|
config keys; updating only one side would leave the split layers inconsistent.
|
|
- `bridge_master.py`, `bridge_reset()`, checks
|
|
`if 'OPTIONS' in CONFIG['SYSTEMS'][_system]['OPTIONS']:` after reset. If the
|
|
system has no `OPTIONS` key, which can happen after disconnect when no
|
|
`_default_options` exists, this raises `KeyError` inside the timed reset loop.
|
|
- `hblink.py` deliberately deletes `CONFIG['SYSTEMS'][system]['OPTIONS']` when a
|
|
peer disconnects or times out and no `_default_options` exists. This confirms
|
|
the missing `OPTIONS` case is part of normal lifecycle behavior, not corrupt
|
|
state.
|
|
|
|
### Assumptions To Validate
|
|
- `GLOBAL.USE_ACL: False` should disable global ACL checks; it should not be
|
|
treated as enabled because the string is truthy.
|
|
- `ALIASES.STALE_DAYS` is intended to control alias refresh age in days, while
|
|
the runtime scheduler should operate on the already-converted seconds value.
|
|
- Periodic alias reloads are intended to replace the live alias dictionaries
|
|
used by bridge logging/reporting and router helper methods.
|
|
- Bridge reset should tolerate sessions with no current `OPTIONS` key and should
|
|
not stop the reactor/timed loops.
|
|
|
|
### Unresolved Questions
|
|
- In `bridge_reset()`, should `_reloadoptions` be set whenever an `OPTIONS` key
|
|
exists, or only when current/default options actually need re-parsing?
|
|
|
|
### Protocol-Sensitive Areas
|
|
- Global ACL parsing affects all packet admission paths.
|
|
- Alias reload affects observability and may affect local subscriber lookup
|
|
helpers, but should not mutate packet bytes.
|
|
- Bridge reset interacts with HBP disconnect/reconnect lifecycle and options
|
|
parsing; fixes must preserve the confirmed session-only option semantics.
|
|
|
|
### Inferred Invariants
|
|
- Boolean config fields must be stored as booleans, not truthy strings.
|
|
- Time values should have one unit conversion boundary.
|
|
- Periodic reload functions must update the state actually read by production
|
|
paths.
|
|
- Timed maintenance loops must not crash on missing optional session keys.
|
|
|
|
### Resolution
|
|
- Updated `config.py` to parse `GLOBAL.USE_ACL` with `getboolean()`.
|
|
- Updated the alias reload scheduler to use `CONFIG['ALIASES']['STALE_TIME']`
|
|
directly, because `config.py` already converts `STALE_DAYS` to seconds.
|
|
- Updated `setAlias()` to assign the reloaded dictionaries to bridge_master
|
|
module globals and the shared `CONFIG` alias keys consumed by `hblink.py`.
|
|
- Updated `bridge_reset()` to check for the presence of the `OPTIONS` key before
|
|
marking `_reloadoptions`.
|
|
- Added deterministic/config coverage for global ACL parsing, alias stale-time
|
|
units, cross-layer alias reload state, and reset after missing session
|
|
`OPTIONS`.
|
|
|
|
## Post-Fix Widened Review Status
|
|
|
|
### Findings
|
|
- A follow-up scan of `config.py`, `bridge_master.py`, `hblink.py`, `API.py`,
|
|
`utils.py`, and `log.py` did not identify another confirmed bug with the same
|
|
confidence as the fixed config/startup/support issues.
|
|
- The current full deterministic suite passes after the widened fixes.
|
|
|
|
### Assumptions
|
|
- Remaining broad `except` blocks and legacy split-layer structure should be
|
|
treated as audit candidates, not bugs, unless a concrete failing path is
|
|
demonstrated.
|
|
|
|
### Unresolved Questions
|
|
- The API layer has not yet had the same level of focused review as packet,
|
|
config, startup, alias, and reset handling.
|
|
- Long-gap HBP/FBP same-stream recovery remains a known improvement candidate,
|
|
not a completed fix.
|
|
|
|
### Protocol-Sensitive Areas
|
|
- API/reset/options changes can affect live session control and should be tested
|
|
carefully if reviewed later.
|
|
- Long-gap recovery must not bypass loop-control, ACL, STUN, source-quench, or
|
|
rate-limiting behavior.
|
|
|
|
### Inferred Invariants
|
|
- Confirmed bugs should have a demonstrated code path and a narrowly scoped
|
|
regression test before production changes are made.
|
|
- Deferred policy or observability work should remain documented separately from
|
|
fixed correctness bugs.
|
|
|
|
## UDP Black-Box Harness Expansion
|
|
|
|
### Findings
|
|
- The UDP black-box layer can sensibly mirror HBP-observable deterministic
|
|
behavior: startup/config parsing that affects admission, HBP registration,
|
|
DMRD routing, packet byte preservation, sequence duplicate/drop behavior,
|
|
terminator lifecycle behavior, and generated prompt output.
|
|
- OBP/FBP coverage remains out of scope for this increment because it requires
|
|
OpenBridge packet/HMAC/protocol-version emulation over UDP.
|
|
|
|
### Assumptions
|
|
- UDP tests should cover high-value end-to-end behavior, not every deterministic
|
|
internal state transition.
|
|
- State-only behavior such as alias dictionary replacement and bridge-reset
|
|
flags remains better covered by the deterministic harness unless an external
|
|
UDP-visible symptom is needed.
|
|
|
|
### Unresolved Questions
|
|
- Add OpenBridge/FBP UDP peer emulation later for enhanced metadata, BCSQ/STUN,
|
|
keepalive, and FBP long-gap behavior.
|
|
- Add recorded packet fixture replay and jitter/drop controls around realistic
|
|
cadence later.
|
|
|
|
### Protocol-Sensitive Areas
|
|
- The current UDP expansion exercises HBP only; it must not be interpreted as
|
|
FBP/OpenBridge black-box coverage.
|
|
- UDP tests assert observable packet fields and bytes, not internal mutable
|
|
status dictionaries.
|
|
|
|
### Inferred Invariants
|
|
- Deterministic tests remain the broad internal regression layer.
|
|
- UDP tests are the external integration confidence layer and should stay
|
|
loopback-only, opt-in, and isolated from real network traffic.
|
|
|
|
### Resolution
|
|
- Added UDP scenario config knobs for global ACL fields and static TG lists.
|
|
- Added opt-in UDP coverage for global ACL false parsing through packet
|
|
admission, HBP data/control payload preservation, voice sequence wrap,
|
|
duplicate sequence `0` suppression, and post-terminator late packet
|
|
suppression.
|
|
- Existing UDP coverage for static routing and local TG9 TS2 dial-a-TG prompt
|
|
output remains in place.
|
|
|
|
## UDP/FBP Black-Box Harness Expansion
|
|
|
|
### Findings
|
|
- `hblink.py` OpenBridge/FBP v5 packets are observable over UDP as `DMRE`
|
|
envelopes with the DMR header/body, BER/RSSI, embedded protocol version,
|
|
timestamp, source server, source repeater, hop count and BLAKE2b hash.
|
|
- Enhanced OpenBridge forwarding is gated by recent `BCKA` state, so a useful
|
|
black-box FBP peer must send signed keepalive traffic before expecting
|
|
forwarded packets.
|
|
- FBP source-quench is represented by signed `BCSQ` packets keyed by TGID and
|
|
stream ID; the UDP harness can assert the external effect by verifying no
|
|
matching `DMRE` leaves FreeDMR for that stream.
|
|
|
|
### Assumptions
|
|
- Initial FBP black-box coverage should focus on protocol-v5 enhanced FBP,
|
|
because that is the current default and carries the richest metadata.
|
|
- The harness may construct valid FBP transport envelopes and bridge-control
|
|
packets, but only production code should perform route-driven DMR field
|
|
rewrites.
|
|
- FBP peer emulation should stay loopback-only and opt-in with the existing UDP
|
|
test gate.
|
|
|
|
### Unresolved Questions
|
|
- Add explicit black-box coverage for `BCST` STUN once desired externally
|
|
observable behaviour is selected for a multi-peer topology.
|
|
- Add older OpenBridge/FBP protocol-version fixtures later so metadata
|
|
assertions follow the protocol version actually negotiated for the session.
|
|
- Add recorded packet replay and jitter/drop controls after the synthetic FBP
|
|
path is stable.
|
|
|
|
### Protocol-Sensitive Areas
|
|
- FBP packet signing must match the exact protocol version layout. Version 5
|
|
hashes bytes through the hop-count field and includes source repeater
|
|
metadata; lower versions differ.
|
|
- FBP inbound network ID is carried in the embedded DMR peer-ID field and must
|
|
match the configured OpenBridge `NETWORK_ID`.
|
|
- Enhanced FBP liveness and source-quench are transport/control state, not DMR
|
|
payload mutation.
|
|
|
|
### Inferred Invariants
|
|
- HBP-to-FBP static TG routing clears the slot bit because OpenBridge traffic is
|
|
carried as TS1 over the FBP transport.
|
|
- FBP-to-HBP static TG routing rewrites only the target HBP slot/TG fields
|
|
required by the active bridge target while preserving stream identity.
|
|
- A peer-requested `BCSQ` for a TG/stream suppresses subsequent outbound FBP
|
|
packets for that TG/stream without affecting unrelated routing.
|
|
|
|
### Resolution
|
|
- Extended `tests/harness/udp_blackbox.py` with FBP constants, packet parsing,
|
|
v5 `DMRE` packet construction, bridge-control signing, generated OBP config
|
|
sections, and a `FbpPeer` loopback emulator.
|
|
- Added opt-in UDP tests for HBP-to-FBP static TG routing, FBP-to-HBP static TG
|
|
routing, FBP source-quench suppression, and inbound FBP network-ID rejection.
|
|
- Updated test and architecture documentation to describe the current FBP UDP
|
|
support and run commands.
|
|
|
|
## UDP Unreliable-Link Simulation
|
|
|
|
### Findings
|
|
- The UDP black-box harness can simulate unreliable links at the fake endpoint
|
|
send boundary without changing FreeDMR production behaviour.
|
|
- FreeDMR's current voice packet-control logic treats DMRD sequence numbers as
|
|
modulo-256, forwards forward progress, and discards delayed out-of-order
|
|
packets rather than buffering and reordering them.
|
|
- FBP trunks can carry arbitrary numbers of streams, so impairment scenarios
|
|
should be able to target one stream without implying that the whole trunk is
|
|
impaired.
|
|
|
|
### Assumptions
|
|
- FreeDMR owns stream ID assignment in the routed/network path; unreliable-link
|
|
tests should not assume a repeater assigns a replacement stream ID after loss.
|
|
- FreeDMR intentionally does not implement a jitter buffer. In real-time AMBE
|
|
stream handling, late or out-of-order packets should generally be discarded
|
|
rather than reconstructed.
|
|
- UDP impairment tests model UDP/IP transport behaviour, not RF late entry,
|
|
AMBE FEC recovery, terminal behaviour or MMDVM jitter buffering.
|
|
|
|
### Unresolved Questions
|
|
- Add burst-loss and blackout scenarios to document current timeout/continuation
|
|
behaviour over longer gaps.
|
|
- Add a whole-trunk impairment warning scenario later if an observable logging
|
|
rule is designed.
|
|
- Add multi-stream FBP impairment where one stream is delayed/reordered while
|
|
unrelated streams continue normally.
|
|
|
|
### Protocol-Sensitive Areas
|
|
- Delayed packets must not override loop-control, STUN, ACLs, source-quench or
|
|
rate-limiting decisions.
|
|
- FBP impairment tests must preserve the correct signed protocol envelope for
|
|
the version under test; the impairment layer schedules transport sends and
|
|
does not mutate DMR or FBP packet bytes.
|
|
|
|
### Inferred Invariants
|
|
- Transport simulation and protocol mutation remain separate.
|
|
- Reordered packets should be observable as missing/dropped packets at the
|
|
destination, not as a buffered corrected sequence.
|
|
- A damaged stream should not poison later streams on the same FBP trunk.
|
|
|
|
### Resolution
|
|
- Added `LinkImpairment` to `tests/harness/udp_blackbox.py` with deterministic
|
|
drop, duplicate, jitter and delay scheduling for fake endpoint sends.
|
|
- Extended HBP and FBP fake endpoints so `send_stream()` / `send_fbp_stream()`
|
|
can apply impairment while preserving packet bytes.
|
|
- Added opt-in UDP tests for delayed out-of-order HBP packets and delayed
|
|
out-of-order FBP packets. Both assert that sequence `1` arriving after
|
|
sequence `2` is not replayed; the FBP test also verifies a following stream on
|
|
the same trunk still routes.
|
|
|
|
## UDP Real-World Scenario Profiles
|
|
|
|
### Findings
|
|
- The UDP black-box suite benefits from reusable scenario profiles because many
|
|
realistic tests need the same 30 ms voice-over packet sequences and impairment
|
|
patterns.
|
|
- Prompt interruption is externally observable over UDP: a generated local TG9
|
|
TS2 prompt should not prevent a real HBP voice stream from routing after the
|
|
prompt has started.
|
|
- FreeDMR is commonly deployed in Docker with the hotspot proxy enabled, but
|
|
proxy/firewall behaviour is a separate integration boundary from direct
|
|
`bridge_master.py` UDP testing.
|
|
|
|
### Assumptions
|
|
- Direct UDP tests should remain the current second layer and should not start
|
|
the hotspot proxy implicitly.
|
|
- Proxy/firewall tests should become a third opt-in layer so direct protocol
|
|
regressions remain isolated from packaging/proxy failures.
|
|
- Any firewall integration test must avoid changing the developer host firewall
|
|
unless it runs in Docker or uses a fake command runner.
|
|
|
|
### Unresolved Questions
|
|
- Add a reliable voice-ident interruption trigger for the subprocess harness.
|
|
The production ident loop currently runs on a long interval, so a direct
|
|
generated-prompt interruption test gives faster coverage now.
|
|
- Decide whether proxy tests should run through Docker Compose, local subprocess
|
|
proxy mode, or both.
|
|
- Inspect external proxy/firewall code from the GitLab repo only when needed and
|
|
with network access explicitly available.
|
|
|
|
### Protocol-Sensitive Areas
|
|
- Stream profiles generate DMR packet sequences; route-driven rewrites still
|
|
belong only to production code.
|
|
- Multi-stream FBP trunk tests should avoid HBP target-slot contention unless
|
|
contention is the behaviour under test.
|
|
- Prompt interruption tests must assert real routed traffic, not just absence of
|
|
prompt packets, because generated speech may have already queued packets.
|
|
|
|
### Inferred Invariants
|
|
- Reusable stream/impairment profiles should be deterministic and named after
|
|
real deployment failure modes where possible.
|
|
- A generated prompt or ident must not permanently block real RF-originated
|
|
voice.
|
|
- Proxy tests are valuable, but they should be opt-in and isolated from the
|
|
direct UDP black-box harness.
|
|
|
|
### Resolution
|
|
- Added `StreamProfile.voice_over()` for reusable 30 ms voice stream packet
|
|
sequences with optional header and terminator packets.
|
|
- Added named `ImpairmentProfiles` for clean links, provider-style reordering,
|
|
mobile flutter drops and duplicate UDP datagrams.
|
|
- Added a UDP prompt-interruption test that observes a local TG9 TS2 generated
|
|
prompt, injects real HBP voice, and verifies the real stream routes to another
|
|
master.
|
|
- Added a multi-stream HBP-to-FBP trunk test where one TG stream is reordered
|
|
and drops its late packet while another clean TG stream continues over the
|
|
same FBP peer.
|
|
- Documented a future third Docker/proxy integration layer for packaged
|
|
deployments and proxy/firewall behaviour.
|
|
|
|
## UDP Hostile Packet Coverage
|
|
|
|
### Findings
|
|
- The UDP black-box harness can exercise malformed and hostile packet paths
|
|
against a real `bridge_master.py` subprocess while keeping all traffic on
|
|
loopback.
|
|
- HBP short `DMRD` datagrams are expected to be ignored without disconnecting
|
|
the emulated repeater; a following valid packet should still route.
|
|
- FBP stale timestamp and max-hop enforcement are expected to source-quench the
|
|
affected TG/stream rather than forwarding to HBP targets.
|
|
|
|
### Assumptions
|
|
- Bad FBP hashes should be ignored without a source-quench because the packet
|
|
did not authenticate.
|
|
- Short malformed FBP packets should be ignored and should not poison the peer
|
|
state for later valid traffic.
|
|
- Source-quench assertions should check the externally visible BCSQ TG/stream
|
|
fields, not internal `_laststrid` state.
|
|
|
|
### Unresolved Questions
|
|
- Add optional subprocess log assertions later for the warning/error messages
|
|
produced by these hostile packet paths.
|
|
- Add bad source-server ID, prohibited OpenBridge slot and old protocol-version
|
|
cases once the next negative-path batch is selected.
|
|
|
|
### Protocol-Sensitive Areas
|
|
- FBP hash corruption must mutate only the transport hash, not the DMR payload,
|
|
so the test isolates authentication handling.
|
|
- Stale timestamp and max-hop tests use valid hashes and valid network IDs so
|
|
they specifically exercise post-auth protocol gates.
|
|
|
|
### Inferred Invariants
|
|
- Malformed/hostile UDP input must not crash the subprocess.
|
|
- Rejected FBP packets must not leak traffic to HBP repeaters.
|
|
- Authenticated but stale/over-hop FBP streams should produce BCSQ for the
|
|
affected TG/stream.
|
|
|
|
### Resolution
|
|
- Added FBP packet-builder options for explicit timestamp and intentionally
|
|
corrupted hash generation.
|
|
- Added `FbpPeer.recv_opcode()` to capture bridge-control responses such as
|
|
BCSQ.
|
|
- Added opt-in UDP tests for short HBP `DMRD`, short FBP `DMRE`, bad FBP hash,
|
|
stale FBP timestamp and max-hop FBP handling.
|
|
|
|
## UDP FBP Bridge-Control Coverage
|
|
|
|
### Findings
|
|
- Enhanced OpenBridge/FBP targets require recent authenticated `BCKA` state
|
|
before HBP-originated traffic is forwarded to them.
|
|
- `BCSQ` is a stream/TG source-quench control and must authenticate before it
|
|
mutates suppression state.
|
|
- `BCST` STUN is a global OpenBridge traffic gate. It blocks FBP send/receive
|
|
paths but does not imply HBP-to-HBP traffic should stop.
|
|
|
|
### Assumptions
|
|
- Invalid bridge-control hashes should be ignored without changing runtime
|
|
state.
|
|
- A valid `BCST` is intended to temporarily stop all FBP/OpenBridge traffic
|
|
until an operator/API path later clears the stun state.
|
|
- Black-box STUN assertions should isolate FBP effects from ordinary HBP bridge
|
|
routing, because valid HBP-to-HBP traffic can still be queued.
|
|
|
|
### Unresolved Questions
|
|
- No un-STUN API path is currently covered; live operator/API semantics remain
|
|
deferred.
|
|
- Older protocol-version and unsupported-version bridge-control cases are still
|
|
future UDP fixture work.
|
|
- Subprocess log assertions for rejected bridge-control packets are still
|
|
optional future coverage.
|
|
|
|
### Protocol-Sensitive Areas
|
|
- Bridge-control HMAC input depends on opcode: `BCVE` signs the version byte,
|
|
while `BCKA`, `BCSQ`, and `BCST` sign their full control bodies.
|
|
- `BCSQ` is scoped to the TGID and stream ID seen on the FBP/OpenBridge side.
|
|
- STUN should not override loop-control or reinterpret delayed stream packets.
|
|
|
|
### Inferred Invariants
|
|
- No authenticated enhanced keepalive means no enhanced HBP-to-FBP forwarding.
|
|
- Invalid `BCSQ` does not suppress any later stream.
|
|
- Valid STUN blocks OpenBridge send/receive while leaving unrelated HBP routing
|
|
behaviour to the normal bridge rules.
|
|
|
|
### Resolution
|
|
- Added corrupt bridge-control hash support to the UDP FBP control builder.
|
|
- Added an invalid BCSQ helper to the fake FBP peer model.
|
|
- Added opt-in UDP tests for enhanced keepalive gating, invalid BCSQ rejection,
|
|
and BCST STUN blocking OpenBridge traffic in both directions.
|
|
|
|
## UDP FBP Protocol-Version Coverage
|
|
|
|
### Findings
|
|
- `BCVE` is the explicit bridge-control version negotiation path. Downgrades,
|
|
unsupported versions and invalid hashes should not change the configured
|
|
outbound packet version.
|
|
- FBP v5 packets carry source repeater metadata. v4 packets use an older layout
|
|
without that field, but v4 is now treated as historical/deprecation context
|
|
rather than a protocol target to preserve long term.
|
|
- v1 remains important and supported as an open OBP interop protocol used by
|
|
other amateur DMR software. The important v1 bridge-instance path is
|
|
`bridge.py`, not primarily `bridge_master.py`.
|
|
- A signed v1 OpenBridge `DMRD` packet received on a v5-configured link is
|
|
refused before normal packet routing and FreeDMR responds with `BCVE`.
|
|
- The UDP harness can build both v4 and v5 envelopes while keeping the inner DMR
|
|
payload bytes generated by the same `PacketSpec`. It can also build signed v1
|
|
OBP packets for refusal tests.
|
|
- `PROTO_VER` is read into `CONFIG['SYSTEMS'][...]['VER']`; historical v4
|
|
behavior is now characterization coverage only. Going forward, expected
|
|
protocol support is v1 OBP where appropriate and v5 FBP.
|
|
- UDP expected-failure coverage now confirms two remaining protocol-version
|
|
issues: unsupported embedded `DMRE` version 6 is not rejected before routing,
|
|
and the v4 send layout currently carries the module default version byte
|
|
instead of the configured `PROTO_VER` value.
|
|
- Recorded packet fixture replay is now available for hex-encoded UDP payloads.
|
|
Replay preserves packet bytes and keeps protocol mutation inside FreeDMR.
|
|
- Subprocess log capture is available for black-box warning/error assertions.
|
|
- True voice-ident black-box interruption remains blocked by the fixed 914
|
|
second production ident loop interval unless a test hook or long-running mode
|
|
is introduced.
|
|
|
|
### Assumptions
|
|
- The generated black-box config uses protocol v5 by default, so failed BCVE
|
|
negotiation should leave outbound packets as v5.
|
|
- Accepting an inbound v4 packet is current behavior, but it is not a desired
|
|
long-term compatibility contract.
|
|
- Refusing v1 on a configured v5 link is the intended behavior because the
|
|
generated test config sets `PROTO_VER` to the current FBP version. This does
|
|
not contradict support for v1 through bridge instances.
|
|
- Protocol options and metadata layout should be asserted against the protocol
|
|
version carried by the packet or negotiated for the session.
|
|
|
|
### Unresolved Questions
|
|
- Decide whether unsupported embedded `DMRE` versions should be rejected at the
|
|
parser seam before routing. The expected-failure test documents the current
|
|
leak.
|
|
- Decide whether the v4 send branch should write the configured protocol
|
|
version byte rather than the module-level `VER` constant. The
|
|
expected-failure test documents the current mismatch; this may become moot if
|
|
v4 is removed.
|
|
- Future v1 interop testing should inspect `bridge.py`, Docker startup files and
|
|
the GitLab wiki for bridge-instance behavior.
|
|
- Add a production-supported fast trigger for voice-ident subprocess tests, or
|
|
keep real ident coverage in deterministic tests and live/manual testing.
|
|
|
|
### Protocol-Sensitive Areas
|
|
- v5 hash input includes the source repeater field and hop byte.
|
|
- v4 hash input omits source repeater and places the hop byte immediately after
|
|
source server metadata.
|
|
- `BCVE` signs only the one-byte version payload, not the opcode.
|
|
- v1 OBP packets use the older `DMRD` envelope and HMAC over the 53-byte packet
|
|
body; a v5-configured receiver refuses them before validating/routing as v1.
|
|
|
|
### Inferred Invariants
|
|
- Invalid or rejected `BCVE` messages do not mutate the outbound protocol
|
|
version.
|
|
- v4 inbound packets can currently route to HBP using the v4 metadata layout,
|
|
but this is characterization/deprecation context.
|
|
- v1 packets on a v5-configured link produce a `BCVE` response and do not route
|
|
to HBP targets.
|
|
- v1 remains supported where a system is intentionally operating as an OBP
|
|
bridge instance.
|
|
- Tests must distinguish bridge-control negotiation from per-packet metadata
|
|
parsing.
|
|
- Recorded fixture replay is transport simulation only; fixture loading must
|
|
not rewrite protocol fields.
|
|
- Log assertions should supplement packet assertions and should not become the
|
|
only evidence of routing behavior.
|
|
|
|
### Resolution
|
|
- Added a version parameter to the UDP FBP packet builder and fake FBP send
|
|
helpers.
|
|
- Added invalid `BCVE` generation to the fake FBP peer model.
|
|
- Added opt-in UDP tests for BCVE downgrade rejection, unsupported BCVE
|
|
rejection, invalid BCVE rejection and inbound v4 packet routing.
|
|
- Added signed v1 OBP packet generation and an opt-in UDP test that verifies v1
|
|
traffic on a v5-configured link is rejected with BCVE and does not leak to HBP.
|
|
- Added a UDP test documenting current v4 downgrade to the older outbound
|
|
layout as characterization/deprecation context.
|
|
- Added expected-failure UDP tests for unsupported embedded FBP packet version
|
|
rejection and configured-v4 version-byte consistency.
|
|
- Added recorded packet fixture replay support and a UDP fixture replay test.
|
|
- Added burst-loss and duplicate-UDP profile coverage for HBP streams.
|
|
- Added subprocess log capture and log assertions for malformed short FBP
|
|
packets and bad FBP hashes.
|
|
|
|
## 2026-05-23 - `bridge.py` Backport Scope
|
|
|
|
### Findings
|
|
- `bridge.py` carries the older conference-bridge voice path and does not
|
|
implement `bridge_master.py` dial-a-TG, data-gateway, generated-prompt or
|
|
configuration-option handling.
|
|
- The shared HBP/OBP group voice packet-control path in `bridge.py` still used
|
|
simple integer comparisons for the one-byte DMRD sequence value, so valid
|
|
modulo-256 forward progress after wrap could be rejected as out-of-order.
|
|
- `bridge.py` HBP slot state retained `lastSeq` and `lastData` across new
|
|
streams on the same slot.
|
|
- `bridge.py` OBP terminator handling only marked `_fin` when live reporting was
|
|
enabled, so late same-stream packets could avoid the finished-stream guard
|
|
when reports were disabled.
|
|
- `bridge.py` HBP terminator handling logged/report-ended streams but did not
|
|
have the finished-stream suppression already added to `bridge_master.py`.
|
|
|
|
### Assumptions
|
|
- Only behavior already present in `bridge.py` should be corrected: group voice
|
|
stream routing, sequence tracking and stream lifecycle.
|
|
- `bridge.py` should not gain dial-a-TG, group data, DATA-GATEWAY, prompt,
|
|
ident, static-TG, default-reflector or broader FBP negotiation features as
|
|
part of this backport.
|
|
- The same modulo-256 sequence policy used by `bridge_master.py` applies to
|
|
`bridge.py` HBP and OBP voice packets because both receive the same DMRD
|
|
sequence byte.
|
|
|
|
### Unresolved Questions
|
|
- A dedicated `bridge.py` runtime harness could be added later if bridge
|
|
instances need the same UDP-level coverage as `bridge_master.py`.
|
|
- `bridge.py` still has older reporting and bridge-rule behavior that was not
|
|
reviewed in this pass.
|
|
|
|
### Protocol-Sensitive Areas
|
|
- DMRD sequence numbers are one byte and wrap at 255 to 0.
|
|
- A modulo delta greater than 127 is treated as old/out-of-order rather than
|
|
forward progress; this preserves the loop-control safety posture discussed
|
|
for `bridge_master.py`.
|
|
- Voice terminator state must suppress late same-stream packets without
|
|
overriding loop-control or adding a jitter buffer.
|
|
|
|
### Inferred Invariants
|
|
- A new HBP stream must start with fresh duplicate/sequence state.
|
|
- OBP finished-stream suppression must not depend on whether the live report
|
|
socket is enabled.
|
|
- `bridge.py` backports should stay limited to bug fixes for behavior the file
|
|
already implements.
|
|
|
|
### Resolution
|
|
- Added `dmrd_seq_delta()` to `bridge.py` and used it for existing HBP/OBP
|
|
duplicate, out-of-order and missed-packet checks.
|
|
- Reset HBP duplicate/sequence state on new streams and after voice
|
|
terminators.
|
|
- Marked OBP streams finished on terminator regardless of reporting state and
|
|
reset per-stream sequence tracking.
|
|
- Added HBP finished-stream state to suppress late same-stream packets after a
|
|
terminator.
|
|
- Added a lightweight `tests/test_bridge_backports.py` check for the shared
|
|
modulo helper and documented the narrower bridge backport coverage.
|
|
|
|
## 2026-05-23 - `API.py` Initial Review
|
|
|
|
### Findings
|
|
- `API.py`, `FD_APIUserDefinedContext.validateKey()`, writes `dmrid` and every
|
|
`peerid` to stdout with `print()`. That bypasses FreeDMR logging and can leak
|
|
API authentication activity into daemon stdout or supervisor logs.
|
|
- `API.py`, `FD_APIUserDefinedContext.getoptions()`, reads
|
|
`CONFIG['SYSTEMS'][system]['OPTIONS']` directly. `hblink.py` deliberately
|
|
deletes that key when an HBP session disconnects and no default options are
|
|
configured, so an authenticated API `getoptions()` can raise `KeyError`
|
|
instead of returning a stable "no current options" value.
|
|
- The former `API.py`, `FD_API.getconfig()` and `FD_API.getbridges()`, declared
|
|
`_returns=Unicode()` but returned the live `CONFIG` and `BRIDGES`
|
|
dictionaries. Those dictionaries contain bytes and nested structures, so the
|
|
declared Spyne return type did not match the actual value.
|
|
- `bridge_master.py`, `kill_server()`, reads
|
|
`CONFIG['GLOBAL']['_KILL_SERVER']` directly, but the key is only set by the
|
|
signal handler or `API.py` `killserver()`. No startup default is visible in
|
|
`config.py`, so the timed kill-server loop can raise `KeyError` before any API
|
|
or signal sets the flag.
|
|
- The former `bridge_master.py`, `config_API()`, accepted `_config` but
|
|
installed `FD_APIUserDefinedContext(CONFIG, _bridges)` using the module
|
|
global. In normal runtime this was probably the same object, but the function
|
|
ignored its parameter and was harder to test in isolation.
|
|
- The former `api_client.py` used Twisted XML-RPC against port 7080, while
|
|
`bridge_master.py` started the Spyne HTTP/JSON API on TCP port 8000. The
|
|
sample client did not match the API server configured by FreeDMR.
|
|
|
|
### Assumptions
|
|
- The API is an optional management surface and should not affect packet routing
|
|
unless an authenticated method mutates live session/config state.
|
|
- User-level API auth is intended to use the options `KEY` associated with the
|
|
connected HBP peer/repeater.
|
|
- Although legacy HBlink can host multiple peers on one master, FreeDMR's
|
|
intended deployment model is one HBP peer per master. Config defaults and
|
|
samples set `MAX_PEERS: 1`, and the proxy maps each external peer ID to its
|
|
own backend destination port/master instance.
|
|
- System-level API auth is intended to use `GLOBAL.SYSTEM_API_KEY`, loaded or
|
|
generated at startup.
|
|
- API `getoptions()` should be safe to call even when a peer has disconnected or
|
|
has no current session options.
|
|
|
|
### Unresolved Questions
|
|
- If config/bridge inspection is reintroduced, it should use bounded,
|
|
JSON-safe snapshots or a separate worker path so it cannot delay voice
|
|
processing in the reactor.
|
|
|
|
### Protocol-Sensitive Areas
|
|
- API-provided `OPTIONS` feeds the same parser as HBP `RPTO` options and can
|
|
change dial-a-TG, static TG, timer, voice-ident and announcement-language
|
|
behavior.
|
|
- API reset toggles `_reset`, which is consumed by `bridge_reset()` and packet
|
|
admission guards.
|
|
- API killserver toggles `_KILL_SERVER`, which is consumed by the Twisted timed
|
|
shutdown loop.
|
|
|
|
### Inferred Invariants
|
|
- API methods should not throw internal exceptions for normal disconnected or
|
|
no-options session states.
|
|
- API authentication details should use FreeDMR logging, not raw stdout.
|
|
- Runtime control flags should have false defaults before timed loops read them.
|
|
- Public API response declarations should match the objects returned.
|
|
|
|
### User-Confirmed API Semantics
|
|
- `getoptions()` should return a clear response when no live options are
|
|
available.
|
|
- API `setoptions()` should receive the full `OPTIONS` string; the API should
|
|
not silently add or preserve `KEY=...`.
|
|
- User-level `reset(dmrid, key)` was intended to act on the matching HBP
|
|
session associated with the supplied DMR ID. Because FreeDMR expects one peer
|
|
per master, the current system-level `_reset` action may be the correct
|
|
implementation once `validateKey()` has proven the peer belongs to that
|
|
master.
|
|
- User confirmed nobody is likely using the experimental Spyne API and replacing
|
|
it now is preferred because Spyne dependency handling is awkward.
|
|
- The API must not delay live voice processing; request handlers should avoid
|
|
blocking work and expensive live-state serialization.
|
|
|
|
### Resolution
|
|
- Replaced the Spyne API layer with a small Twisted HTTP/JSON resource in
|
|
`API.py`.
|
|
- Removed Spyne imports from `bridge_master.py` and removed Spyne from
|
|
`requirements.txt`.
|
|
- Kept API operations to small in-memory control-plane mutations: version,
|
|
health, reset, options get/set, system kill and resetall.
|
|
- Removed live `getconfig()`/`getbridges()` API endpoints to avoid potentially
|
|
expensive serialization in the voice process reactor.
|
|
- Added a small request-body limit so API calls cannot submit large JSON bodies
|
|
that would delay the reactor.
|
|
- Added a safe `_KILL_SERVER` default and changed the shutdown loop to read the
|
|
flag with `.get()`.
|
|
- Updated the sample API client to use HTTP/JSON on port 8000.
|
|
|
|
## 2026-05-23 - Support Module Review, Batch 1
|
|
|
|
### Findings
|
|
- `AMI.py` appears to be live when `ALLSTAR.ENABLED` is true: `bridge_master.py`
|
|
creates `AMIOBJ` and dial-a-TG AllStar control calls `AMIOBJ.send_command()`.
|
|
- `bridge_master.py` logs the configured AllStar password when setting up AMI.
|
|
If AllStar is current, this is a credential exposure bug.
|
|
- `AMI.py`, `AMIClient.lineReceived()`, prints every AMI response line directly
|
|
to stdout instead of using the FreeDMR logger. This can leak operational
|
|
details and makes service logging inconsistent.
|
|
- `AMI.py` stores command and credentials on the nested `AMIClient` class object
|
|
before connecting. Closely spaced `send_command()` calls could overwrite each
|
|
other's command state before the TCP client sends it.
|
|
- `AMI.py`, `AMI.closeConnection()`, references `self.transport`, but the outer
|
|
`AMI` object is not the protocol instance and does not own that attribute.
|
|
- `utils.py`, `try_download()`, disables TLS certificate verification for alias
|
|
downloads with `ssl._create_unverified_context()`. This trades compatibility
|
|
for integrity risk on configured HTTPS alias/checksum URLs.
|
|
- `const.py` defines `ID_MAX = 16776415`, while `bridge_master.py` now uses
|
|
`DMR_ID_MAX = 16777215` for dial-a-TG validation. Because `config.py` uses
|
|
`const.ID_MAX` for ACL building, max-ID handling may be inconsistent.
|
|
- `read_ambe.py`, `readAMBE.readfiles()`, returns `False` on missing voice pack
|
|
files, but startup later iterates `words.keys()`. A missing configured audio
|
|
language can fail startup with a secondary attribute error rather than a clear
|
|
"missing voice pack" error.
|
|
- `mk_voice.py`, `pkt_gen()`, still uses a random stream ID for generated voice
|
|
prompts. Collision probability is low, but generated prompts share the live
|
|
stream namespace.
|
|
|
|
### Assumptions
|
|
- AllStar/AMI support is optional but still intended to work when enabled.
|
|
- Alias downloads are operational convenience data, but stale or modified alias
|
|
data can affect dashboard/reporting clarity and possibly server ID metadata.
|
|
- Audio packs may be provided by deployment packaging even when not obvious from
|
|
minimal test fixtures.
|
|
|
|
### Unresolved Questions
|
|
- Was disabled TLS verification for alias downloads intentional because of old
|
|
CA or embedded-platform compatibility problems?
|
|
- Is `const.ID_MAX = 16776415` intentional, or should shared ACL validation use
|
|
the DMR 24-bit maximum `16777215`?
|
|
- Should missing configured voice prompt languages be a clear fatal startup
|
|
error, or should FreeDMR fall back to a known shipped language?
|
|
|
|
### User-Confirmed Status
|
|
- AllStar/AMI support is current enough to tidy up.
|
|
|
|
### Protocol-Sensitive Areas
|
|
- AllStar control is driven from dial-a-TG private-call handling and must not
|
|
block or delay voice packet processing.
|
|
- Alias and server ID files are read at startup and may influence reporting,
|
|
validation and operational visibility.
|
|
- DMR ID and TG range constants must match the 24-bit field constraints unless a
|
|
narrower range is deliberately reserved by FreeDMR policy.
|
|
- Voice prompt packet generation shares packet timing, TG/slot routing and
|
|
stream lifecycle semantics with live DMRD traffic.
|
|
|
|
### Inferred Invariants
|
|
- Runtime logs must not expose configured passphrases or management credentials.
|
|
- Optional external control paths should use FreeDMR logging and reactor-safe
|
|
Twisted patterns.
|
|
- Startup configuration failures should be explicit enough for sysops to fix
|
|
without packet-level debugging.
|
|
|
|
## 2026-05-23 - Auxiliary Script Review, Batch 2
|
|
|
|
### Findings
|
|
- `app_template.py` and `blank_app.py` compile and look like HBlink example
|
|
scaffolding rather than current FreeDMR packet-routing services.
|
|
- `bridge_all.py` and `bridge_all_master.py` compile, but their ACL checks use
|
|
`TG1_ACL` and `TG2_ACL`. Current config parsing creates `TGID_TS1_ACL` and
|
|
`TGID_TS2_ACL`, so these scripts appear stale if they are still run.
|
|
- `bridge_all.py` and `bridge_all_master.py` use the older sequence-loss check
|
|
that was already noted as rollover-sensitive in code comments.
|
|
- `report_receiver.py` and `report_sql.py` are external reporting clients and
|
|
use `pickle.loads()` on reporting socket payloads. This is acceptable only if
|
|
the report source is trusted and local/private.
|
|
- `report_sql.py` inserts report events by formatting SQL strings directly.
|
|
Event fields originate from the reporting socket and should be parameterized
|
|
if this client is current.
|
|
- `playback.py`, `playback_file.py` and `play_ambe.py` compile, but they use
|
|
blocking `sleep()` calls in Twisted callbacks or packet receive paths. These
|
|
are suitable for lab/playback utilities, not live voice-process services.
|
|
- `docker-configs/supervisord.conf` starts `playback.py` as a supervised
|
|
process by default in the proxy image, so the blocking-playback concern may
|
|
matter in packaged deployments if that program is not intentionally enabled.
|
|
- `hotspot_proxy_v2.py` is current enough to matter: `hblink.py` has explicit
|
|
`PRIN` and `PRBL` handling and sample HBP config enables `PROXY_CONTROL`.
|
|
- `hotspot_proxy_v2.py` installs a SIGTERM handler named `sigt()` that only
|
|
prints `oooh` and does not stop the reactor. Under supervisor/container
|
|
shutdown, the proxy may not terminate cleanly on SIGTERM.
|
|
- `hotspot_proxy_v2.py` falls back to an internal default configuration if any
|
|
required `[PROXY]` option is missing or invalid. That can start a proxy on
|
|
default ports after a config typo instead of failing closed.
|
|
- `hdstack/hotspot_proxy_v2.py` is a separate older proxy copy with materially
|
|
different behavior and one visible typo path (`_data` in DMRA handling), but
|
|
it may be legacy or a special HDStack deployment copy.
|
|
- Several Dockerfiles still build from GitHub while `Dockerfile-ci` copies the
|
|
local tree. Some images therefore may not include local checkout changes when
|
|
built from this repository directory.
|
|
- `docker-configs/Dockerfile-hbmonv2` creates user `hbmon` but runs
|
|
`chown -R radio: /opt/HBMonv2`; if this Dockerfile is current, that user name
|
|
mismatch can fail image build.
|
|
|
|
### Assumptions
|
|
- The auxiliary HBlink example scripts are less important than
|
|
`bridge_master.py`, `hblink.py`, `bridge.py`, `config.py`, proxy and Docker
|
|
packaging.
|
|
- The reporting socket is normally intended for trusted dashboards/clients, but
|
|
it may still be exposed on configured TCP interfaces.
|
|
- The top-level `hotspot_proxy_v2.py` is the main packaged proxy; the `hdstack`
|
|
copy may exist for a special multi-instance deployment.
|
|
|
|
### Unresolved Questions
|
|
- Should Dockerfiles that clone from GitHub be kept, or should maintained
|
|
Docker builds copy the local source tree like `Dockerfile-ci`?
|
|
|
|
### User-Confirmed Status
|
|
- `bridge_all.py` and `bridge_all_master.py` are legacy/experimental; leave
|
|
them for now.
|
|
- `playback.py`, `playback_file.py` and `play_ambe.py` are a combination of lab
|
|
tools, experimental code and legacy code; they are low priority.
|
|
- `report_receiver.py` and `report_sql.py` are lightly used/current: they feed
|
|
`https://freedmr-lh.gb7fr.org.uk/` in one deployment.
|
|
- `hdstack/hotspot_proxy_v2.py` is experimental capacity work; leave detailed
|
|
review for later.
|
|
|
|
### Follow-Up Findings For Current Reporting Clients
|
|
- `report_sql.py`, `send_mysql()`, performs reconnect attempts and MySQL writes
|
|
directly in the Twisted reactor path. If the database stalls or reconnects
|
|
slowly, the reporting client can stop processing its TCP report stream.
|
|
- `report_sql.py`, `send_mysql()`, uses string formatting to build the `insert
|
|
into feed` SQL statement. Even on a trusted report stream, quotes or commas in
|
|
fields can break inserts; parameterized SQL would be safer and simpler.
|
|
- `report_sql.py`, `send_mysql()`, closes the cursor only on error, not after
|
|
successful inserts.
|
|
- `report_sql.py`, `reportClientFactory.buildProtocol()`, returns
|
|
`self.proto(db, reactor)` using module globals rather than `self.db` and
|
|
`self.reactor`. It works in the current `__main__` path but makes the factory
|
|
unnecessarily fragile.
|
|
- `report_receiver.py` CLI flags such as `--events 0`, `--config 0` and
|
|
`--bridges 0` are parsed as strings, so `"0"` is truthy and enables the
|
|
output. These should be real booleans or checked against `"1"`.
|
|
- Both report clients use `pickle.loads()` for config/bridge snapshots. User has
|
|
confirmed current use is a known deployment, so this should be documented as a
|
|
trusted-report-socket assumption rather than changed blindly.
|
|
|
|
### Protocol-Sensitive Areas
|
|
- Proxy session mapping is part of the intended one-HBP-peer-per-master
|
|
architecture and must preserve HBP registration/control packet ordering.
|
|
- Proxy `PRIN` and `PRBL` packets affect client IP awareness and dynamic
|
|
firewall/source-quench behavior.
|
|
- Playback utilities synthesize or replay DMRD voice frames and can disturb
|
|
timing assumptions if run in the same reactor as live services.
|
|
- Reporting clients consume the same live-report stream used by dashboards;
|
|
field shape changes can break external consumers.
|
|
|
|
### Inferred Invariants
|
|
- Long-running daemons under Docker/supervisor should exit cleanly on SIGTERM.
|
|
- Packaged default process lists should avoid optional lab tools unless the
|
|
deployment intentionally enables them.
|
|
- Config parse failures for network-facing daemons should fail closed rather
|
|
than silently opening default listeners.
|
|
|
|
## 2026-05-23 - Packaging, Config and Documentation Review, Batch 3
|
|
|
|
### Findings
|
|
- `rules_SAMPLE.py` is an empty static-routing skeleton and compiles.
|
|
- `systemd-scripts/freedmrrepeater.service` appears stale: it uses
|
|
`/opt/FreeDMR` and `./config/hblink.cfg`, while current sample configs and
|
|
Docker entrypoints run from `/opt/freedmr` with `freedmr.cfg`.
|
|
- `pyvenv.cfg` is tracked at repository root. This makes the source checkout
|
|
look like a Python virtual environment and can confuse tooling or developers.
|
|
- Current FreeDMR/Docker config samples mostly use current ACL names
|
|
`TGID_TS1_ACL` and `TGID_TS2_ACL`.
|
|
- `FreeDMR-SAMPLE.cfg` and `FreeDMR-SAMPLE-commented.cfg` still show
|
|
`PROTO_VER: 2`, while Docker config documents `PROTO_VER: 5` for FreeDMR FBP
|
|
and `PROTO_VER: 1` for OBP/external software.
|
|
- `hblink-SAMPLE.cfg` is intentionally HBlink-flavored and lacks current
|
|
FreeDMR-only defaults such as API, dial-a-TG option defaults and proxy
|
|
control.
|
|
- `hdstack/*.cfg`, `loro.cfg` and `playback_file.cfg` appear special-purpose
|
|
configs rather than main FreeDMR defaults.
|
|
- `docker-configs/Dockerfile-noproxy`, `Dockerfile-proxy` and
|
|
`Dockerfile-hdstack` clone from remote GitHub rather than copying the local
|
|
tree, so building them from this checkout may not include current local
|
|
changes. `Dockerfile-ci` does copy the local source tree.
|
|
- `docker-configs/entrypoint-proxy` accepts `BRIDGE_SERVER=1` and runs
|
|
`bridge.py -c freedmr.cfg -r rules.py`; `bridge.py` supports `-r`, so this
|
|
entrypoint matches the maintained bridge path.
|
|
- `docs/api.md`, `docs/testing.md` and `docs/test-harness-design.md` reflect
|
|
the new HTTP/JSON API and current deterministic/UDP harness split.
|
|
|
|
### Assumptions
|
|
- Docker is the preferred general deployment path.
|
|
- `Dockerfile-ci` is the maintained local-build path for this working tree.
|
|
- `hblink-SAMPLE.cfg` remains useful as an upstream-style reference and should
|
|
not be forced into FreeDMR master semantics unless the project wants that.
|
|
|
|
### Unresolved Questions
|
|
- Should the non-CI Dockerfiles be kept as remote-clone recipes, or updated to
|
|
copy local source for reviewable builds?
|
|
- Should `FreeDMR-SAMPLE*.cfg` be updated from `PROTO_VER: 2` to the current
|
|
documented FreeDMR default of `5`, while retaining notes that OBP/external
|
|
bridges use `1`?
|
|
|
|
### User-Confirmed Status
|
|
- `report_receiver.py` and `report_sql.py` are kind-of current: they are used in
|
|
one deployment to feed `https://freedmr-lh.gb7fr.org.uk/`.
|
|
- `hdstack/hotspot_proxy_v2.py` is experimental capacity work; defer until the
|
|
capacity design is discussed.
|
|
- Disabled TLS verification in alias downloads is probably intentional; leave
|
|
behavior unchanged for now.
|
|
- Docker containers are the recommended deployment path because they reduce
|
|
support overhead; the systemd unit may exist but is not the main focus.
|
|
- Root `pyvenv.cfg` probably should not be tracked.
|
|
|
|
### Protocol-Sensitive Areas
|
|
- `PROTO_VER` controls OBP/FBP packet option ordering and available fields; test
|
|
fixtures must match the negotiated/configured protocol version.
|
|
- The service manager and container entrypoints define which components are
|
|
actually live in packaged deployments, including proxy and optional playback.
|
|
|
|
### Inferred Invariants
|
|
- Maintained sample configs should not encourage obsolete FBP protocol versions
|
|
unless backward compatibility is being intentionally demonstrated.
|
|
- Runtime packaging should start the same code and config layout that users are
|
|
expected to operate.
|
|
|
|
### Resolution
|
|
- Tidied current AllStar/AMI support without changing dial-a-TG packet logic:
|
|
`bridge_master.py` now redacts the configured AMI password in startup logs,
|
|
`AMI.py` logs AMI responses through the module logger instead of printing raw
|
|
lines, and AMI command/auth state is held on protocol instances rather than
|
|
shared on the protocol class.
|
|
- Changed `AMI.closeConnection()` to disconnect the Twisted connector it owns
|
|
instead of referencing a nonexistent outer `transport` attribute.
|
|
- Updated `report_sql.py` so report events schedule database writes with
|
|
`reactor.callInThread()`, use a lock around the shared DB connection, execute
|
|
the existing feed insert with parameterized SQL, close cursors in `finally`,
|
|
and build clients with the factory's `self.db` / `self.reactor`.
|
|
- Updated `report_receiver.py` so CLI flags such as `--events 0`,
|
|
`--config 0`, `--bridges 0`, and `--stats 0` parse as false.
|
|
- Removed tracked root `pyvenv.cfg`.
|
|
- Added `tests/test_auxiliary_tools.py` for AMI protocol state, report receiver
|
|
flag parsing and report SQL parameterized insert behavior.
|
|
- Updated `docs/testing.md` with the auxiliary utility coverage.
|
|
|
|
## 2026-05-23 - Proxy And Shared Constant Review
|
|
|
|
### Findings
|
|
- `hotspot_proxy_v2.py` parsed environment booleans with Python `bool()`.
|
|
Because `bool("0")` is true, Docker settings such as `FDPROXY_IPV6=0` enabled
|
|
the option instead of disabling it.
|
|
- `hotspot_proxy_v2.py` installed a SIGTERM handler that only printed `oooh`.
|
|
Under Docker/supervisor shutdown, the proxy could ignore the normal
|
|
termination signal until forcibly stopped.
|
|
- `hotspot_proxy_v2.py` falls back to an internal default config when `[PROXY]`
|
|
is absent or invalid. This looked risky in isolation, but the current Docker
|
|
config files do not include a `[PROXY]` section, so changing this would alter
|
|
deployment behavior and needs a packaging decision.
|
|
- `const.ID_MAX` remains `16776415` while other current code treats the DMR
|
|
24-bit all-call value `16777215` as the upper reserved boundary. The narrower
|
|
shared ACL max may be intentional reservation or a historical typo; user was
|
|
not sure, so it is left unchanged.
|
|
|
|
### Assumptions
|
|
- The top-level `hotspot_proxy_v2.py` is the current packaged proxy.
|
|
- Docker environment variables named with `=0` are intended to disable the
|
|
feature.
|
|
- The proxy's default internal configuration is currently part of Docker
|
|
startup behavior when no `[PROXY]` section is supplied.
|
|
|
|
### Unresolved Questions
|
|
- Should proxy config fallback remain the default Docker behavior, or should
|
|
Docker configs grow an explicit `[PROXY]` section before the fallback is made
|
|
stricter?
|
|
- Is `const.ID_MAX = 16776415` a deliberate FreeDMR policy limit, or should ACL
|
|
validation use the full 24-bit DMR maximum/reserved all-call boundary?
|
|
|
|
### Protocol-Sensitive Areas
|
|
- Proxy `PRIN` and `PRBL` are part of client-source awareness and dynamic
|
|
blocklisting; changes here affect HBP client admission and abuse handling.
|
|
- ACL ID maxima affect which DMR IDs and TGs can be expressed in config, even
|
|
when packet parsing itself can carry the full 24-bit field.
|
|
|
|
### Inferred Invariants
|
|
- `FDPROXY_*` environment booleans should parse like config booleans, not Python
|
|
truthiness of non-empty strings.
|
|
- Containerized long-running daemons should handle SIGTERM as graceful shutdown.
|
|
|
|
### Resolution
|
|
- Added `bool_from_env()` in `hotspot_proxy_v2.py` and applied it to
|
|
`FDPROXY_IPV6`, `FDPROXY_STATS`, `FDPROXY_CLIENTINFO`, and the commented debug
|
|
override path.
|
|
- Changed the proxy SIGTERM handler to use the same graceful shutdown handler as
|
|
SIGINT.
|
|
- Added auxiliary test coverage for proxy environment boolean parsing.
|