mirror of
https://github.com/n0mjs710/dmrlink3
synced 2026-11-08 04:19:24 +00:00
trunk: STATUS is {}; routing core skips TX/RX writes for trunk systems
A TRUNK system carries an arbitrary number of simultaneous streams, so
the previous TS-keyed STATUS dict was semantically wrong and never
actually read by any code (contention checks are bypassed for trunk
targets via TRUNKS list).
- trunk.py TRUNK.__init__: self.STATUS = {} with explanatory comment
- bridge.py bridgeIPSC.group_voice: wrap post-transmit TX state writes
in `if _target['SYSTEM'] not in TRUNKS` — trunk targets have no TS
slots to write into, and the values would never be consulted
- bridge.py bridgeTRUNK.group_voice: same TX guard + remove the final
two RX state writes (self.STATUS[_ts]['RX_TGID/RX_TIME']) that would
KeyError against the now-empty STATUS dict
13 new tests in tests/test_trunk_status.py all pass. The 33
pre-existing failures in test_bridge.py / test_protocol.py (missing
SUB_ACL in test helpers) are unchanged.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
Cort BuffingtonandClaude Sonnet 4.6 committed
1 parent
8d5307f4c3
commit
94e145ec7c
4 files changed
+271
-20
No files matched your search
@@ -339,9 +339,10 @@ class bridgeIPSC(IPSC):
|
||||
_src_sub, _target['TGID'], _target['TS'], _burst_data_type, _tmp_data, _peerid)
|
||||
# END FRAME FORWARDING
|
||||
|
||||
_target_status[_target['TS']]['TX_TGID'] = _target['TGID']
|
||||
_target_status[_target['TS']]['TX_TIME'] = now
|
||||
_target_status[_target['TS']]['TX_SRC_SUB'] = _src_sub
|
||||
if _target['SYSTEM'] not in TRUNKS:
|
||||
_target_status[_target['TS']]['TX_TGID'] = _target['TGID']
|
||||
_target_status[_target['TS']]['TX_TIME'] = now
|
||||
_target_status[_target['TS']]['TX_SRC_SUB'] = _src_sub
|
||||
|
||||
# Record RX state for contention handler
|
||||
self.STATUS[_ts]['RX_TGID'] = _dst_group
|
||||
@@ -480,12 +481,13 @@ class bridgeTRUNK(TRUNK):
|
||||
systems[_target['SYSTEM']].transmit_group_voice(
|
||||
_src_sub, _target['TGID'], _target['TS'], _burst_data_type, _tmp_data, _peerid)
|
||||
|
||||
_target_status[_target['TS']]['TX_TGID'] = _target['TGID']
|
||||
_target_status[_target['TS']]['TX_TIME'] = now
|
||||
_target_status[_target['TS']]['TX_SRC_SUB'] = _src_sub
|
||||
if _target['SYSTEM'] not in TRUNKS:
|
||||
_target_status[_target['TS']]['TX_TGID'] = _target['TGID']
|
||||
_target_status[_target['TS']]['TX_TIME'] = now
|
||||
_target_status[_target['TS']]['TX_SRC_SUB'] = _src_sub
|
||||
|
||||
self.STATUS[_ts]['RX_TGID'] = _dst_group
|
||||
self.STATUS[_ts]['RX_TIME'] = now
|
||||
# Trunk STATUS is {} — RX state writes are omitted; values are only
|
||||
# consulted by contention checks, which are bypassed for trunk targets.
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@@ -224,6 +224,32 @@ def make_gv_packet(peer_id, src_sub, dst_group, burst_type, timeslot, seq=0):
|
||||
)
|
||||
|
||||
|
||||
def make_trunk_system(radio_id_int=3129100, ip='127.0.0.1', port=50200,
|
||||
peer_ip='10.0.0.1', peer_port=50200):
|
||||
"""Return a SYSTEMS entry for a TRUNK system (parallel to make_system).
|
||||
|
||||
Matches the structure produced by config.py when SYSTEM_TYPE = TRUNK.
|
||||
"""
|
||||
return {
|
||||
'SYSTEM_TYPE': 'TRUNK',
|
||||
'LOCAL': {
|
||||
'ENABLED': True,
|
||||
'SYSTEM_TYPE': 'TRUNK',
|
||||
'RADIO_ID': bytes_4(radio_id_int),
|
||||
'IP': ip,
|
||||
'PORT': port,
|
||||
'GROUP_HANGTIME': 5,
|
||||
},
|
||||
'TRUNK': {
|
||||
'PEER_IP': peer_ip,
|
||||
'PEER_PORT': peer_port,
|
||||
'PEER_SOCK': (peer_ip, peer_port),
|
||||
},
|
||||
'MASTER': {},
|
||||
'PEERS': {},
|
||||
}
|
||||
|
||||
|
||||
def make_bridge_rules(src_system, tgt_system, src_tgid, tgt_tgid=None, ts=1, active=True):
|
||||
"""Return a BRIDGES dict with one bridge connecting src_system to tgt_system."""
|
||||
if tgt_tgid is None:
|
||||
|
||||
@@ -0,0 +1,226 @@
|
||||
"""
|
||||
Tests for Trunk v2 STATUS write semantics.
|
||||
|
||||
A TRUNK system can carry an arbitrary number of simultaneous streams, so its
|
||||
STATUS dict cannot be TS-keyed without lying about capacity. More importantly,
|
||||
the TS-keyed values are never consulted for any decision: contention checks are
|
||||
bypassed for trunk targets (TRUNKS list), and no other code reads trunk STATUS.
|
||||
|
||||
Expected behaviour after the fix:
|
||||
- TRUNK.STATUS is {} (empty dict, no TS keys)
|
||||
- Routing FROM an IPSC source TO a trunk target does not write TX state
|
||||
- Routing FROM a trunk source does not write RX state to the trunk's STATUS
|
||||
- Routing FROM a trunk source TO an IPSC target still writes TX state (IPSC
|
||||
targets do use STATUS for contention), and the packet is still forwarded
|
||||
"""
|
||||
|
||||
import unittest
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
import dmrlink
|
||||
import bridge
|
||||
from bridge import bridgeIPSC, bridgeTRUNK
|
||||
from const import VOICE_HEAD, VOICE_TERM, SLOT1_VOICE
|
||||
from dmr_utils3.utils import bytes_3, bytes_4
|
||||
|
||||
from helpers import (
|
||||
MockDatagramTransport, make_config, make_system, make_trunk_system,
|
||||
make_gv_packet, make_bridge_rules,
|
||||
)
|
||||
|
||||
# Pre-built PERMIT:ALL ACL tuple — what config.acl_build('PERMIT:ALL', ID_MAX)
|
||||
# returns. Avoids importing config just for a constant.
|
||||
_PERMIT_ALL = (True, frozenset(), (1,), (16776415,))
|
||||
|
||||
MASTER_ID = 3112000
|
||||
TRUNK_RID = 3129100
|
||||
EXT_ID = 3112010
|
||||
EXT_ID_B = bytes_4(EXT_ID)
|
||||
SRC_SUB = b'\x00\x00\x01'
|
||||
TGID_9 = bytes_3(9)
|
||||
|
||||
|
||||
def _make_config():
|
||||
"""Two-system config: one IPSC master and one trunk endpoint."""
|
||||
cfg = make_config({
|
||||
'TEST-MASTER': make_system(MASTER_ID, '127.0.0.1', 50100, master_peer=True),
|
||||
'TEST-TRUNK': make_trunk_system(TRUNK_RID),
|
||||
})
|
||||
# bridgeIPSC.group_voice() and bridgeTRUNK.group_voice() both call
|
||||
# acl_check() against GLOBAL.SUB_ACL; supply PERMIT:ALL so the tests
|
||||
# reach the forwarding code.
|
||||
cfg['GLOBAL']['SUB_ACL'] = _PERMIT_ALL
|
||||
return cfg
|
||||
|
||||
|
||||
class BridgeTrunkBase(unittest.TestCase):
|
||||
"""
|
||||
Base: IPSC master + TRUNK endpoint, bridge MASTER→TRUNK on TGID 9.
|
||||
Trunk is in TRUNKS so contention is bypassed.
|
||||
BRIDGE_SRC_INDEX / BRIDGE_BY_SYSTEM are built from BRIDGES so the
|
||||
forwarding loop actually executes.
|
||||
"""
|
||||
|
||||
def setUp(self):
|
||||
self.config = _make_config()
|
||||
self.master = bridgeIPSC('TEST-MASTER', self.config, None)
|
||||
self.trunk = bridgeTRUNK('TEST-TRUNK', self.config, None)
|
||||
|
||||
self.master.send_to_ipsc = MagicMock()
|
||||
self.trunk.transport = MockDatagramTransport()
|
||||
|
||||
self._orig_systems = dict(dmrlink.systems)
|
||||
self._orig_bridges = bridge.BRIDGES
|
||||
self._orig_trunks = bridge.TRUNKS
|
||||
self._orig_src_index = bridge.BRIDGE_SRC_INDEX
|
||||
self._orig_by_system = bridge.BRIDGE_BY_SYSTEM
|
||||
|
||||
dmrlink.systems.clear()
|
||||
dmrlink.systems.update({'TEST-MASTER': self.master, 'TEST-TRUNK': self.trunk})
|
||||
|
||||
bridge.BRIDGES = make_bridge_rules('TEST-MASTER', 'TEST-TRUNK', TGID_9)
|
||||
bridge.TRUNKS = ['TEST-TRUNK']
|
||||
bridge.BRIDGE_SRC_INDEX, bridge.BRIDGE_BY_SYSTEM = bridge.index_bridges(bridge.BRIDGES)
|
||||
|
||||
def tearDown(self):
|
||||
dmrlink.systems.clear()
|
||||
dmrlink.systems.update(self._orig_systems)
|
||||
bridge.BRIDGES = self._orig_bridges
|
||||
bridge.TRUNKS = self._orig_trunks
|
||||
bridge.BRIDGE_SRC_INDEX = self._orig_src_index
|
||||
bridge.BRIDGE_BY_SYSTEM = self._orig_by_system
|
||||
|
||||
def _gv(self, system, burst_type, ts=1, src=SRC_SUB, dst=TGID_9):
|
||||
pkt = make_gv_packet(EXT_ID_B, src, dst, burst_type, ts)
|
||||
system.group_voice(src, dst, ts, False, EXT_ID_B, pkt)
|
||||
|
||||
def _reverse_bridge(self):
|
||||
"""Switch the bridge so trunk is the source and master is the target."""
|
||||
bridge.BRIDGES = make_bridge_rules('TEST-TRUNK', 'TEST-MASTER', TGID_9)
|
||||
bridge.BRIDGE_SRC_INDEX, bridge.BRIDGE_BY_SYSTEM = bridge.index_bridges(bridge.BRIDGES)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# STATUS structure
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
class TestTrunkStatusStructure(BridgeTrunkBase):
|
||||
"""TRUNK.STATUS must be an empty dict with no TS keys."""
|
||||
|
||||
def test_status_is_empty_dict(self):
|
||||
self.assertEqual(self.trunk.STATUS, {})
|
||||
|
||||
def test_status_has_no_slot1_key(self):
|
||||
self.assertNotIn(1, self.trunk.STATUS)
|
||||
|
||||
def test_status_has_no_slot2_key(self):
|
||||
self.assertNotIn(2, self.trunk.STATUS)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# IPSC source → trunk target: TX state must not be written
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
class TestIpscToTrunkTxState(BridgeTrunkBase):
|
||||
"""
|
||||
When an IPSC source routes to a trunk target, TX state must not be
|
||||
written to the trunk's STATUS dict (it has no TS keys and the values
|
||||
would never be read for any contention decision anyway).
|
||||
"""
|
||||
|
||||
def test_tx_state_not_written_on_voice_head(self):
|
||||
self._gv(self.master, VOICE_HEAD)
|
||||
self.assertEqual(self.trunk.STATUS, {},
|
||||
'TX state must not be written to trunk STATUS on VOICE_HEAD')
|
||||
|
||||
def test_tx_state_not_written_on_interior_burst(self):
|
||||
self._gv(self.master, VOICE_HEAD)
|
||||
self._gv(self.master, SLOT1_VOICE)
|
||||
self.assertEqual(self.trunk.STATUS, {},
|
||||
'TX state must not be written to trunk STATUS on interior burst')
|
||||
|
||||
def test_tx_state_not_written_on_voice_term(self):
|
||||
self._gv(self.master, VOICE_HEAD)
|
||||
self._gv(self.master, VOICE_TERM)
|
||||
self.assertEqual(self.trunk.STATUS, {},
|
||||
'TX state must not be written to trunk STATUS on VOICE_TERM')
|
||||
|
||||
def test_full_call_to_trunk_no_crash(self):
|
||||
"""Full call routed to trunk must not raise KeyError at any point."""
|
||||
try:
|
||||
self._gv(self.master, VOICE_HEAD)
|
||||
for _ in range(3):
|
||||
self._gv(self.master, SLOT1_VOICE)
|
||||
self._gv(self.master, VOICE_TERM)
|
||||
except KeyError as exc:
|
||||
self.fail(f'Routing to trunk raised KeyError: {exc}')
|
||||
|
||||
def test_packet_is_still_forwarded_to_trunk(self):
|
||||
"""TX state skip must not stop the packet being sent to the trunk peer."""
|
||||
self._gv(self.master, VOICE_HEAD)
|
||||
self.assertTrue(self.trunk.transport.sent,
|
||||
'VOICE_HEAD must still be forwarded to trunk peer')
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Trunk source → IPSC target: TX state MUST still be written
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
class TestTrunkToIpscTxState(BridgeTrunkBase):
|
||||
"""
|
||||
When a trunk source routes to an IPSC target, TX state MUST be written
|
||||
to the IPSC target's STATUS (IPSC targets use it for contention checks).
|
||||
"""
|
||||
|
||||
def setUp(self):
|
||||
super().setUp()
|
||||
self._reverse_bridge()
|
||||
|
||||
def test_tx_tgid_written_to_ipsc_target(self):
|
||||
self._gv(self.trunk, VOICE_HEAD)
|
||||
self.assertEqual(self.master.STATUS[1]['TX_TGID'], TGID_9,
|
||||
'TX_TGID must be written to IPSC target STATUS')
|
||||
|
||||
def test_tx_time_written_to_ipsc_target(self):
|
||||
self._gv(self.trunk, VOICE_HEAD)
|
||||
self.assertGreater(self.master.STATUS[1]['TX_TIME'], 0,
|
||||
'TX_TIME must be written to IPSC target STATUS')
|
||||
|
||||
def test_packet_forwarded_to_ipsc_target(self):
|
||||
self._gv(self.trunk, VOICE_HEAD)
|
||||
self.master.send_to_ipsc.assert_called_once()
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Trunk source RX state: must not be written back to trunk STATUS
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
class TestTrunkSourceRxState(BridgeTrunkBase):
|
||||
"""
|
||||
When trunk is the source, the routing core must not write RX state back
|
||||
to the trunk's own STATUS dict (it has no TS keys, and the values would
|
||||
never be read for contention since trunk targets bypass the check).
|
||||
"""
|
||||
|
||||
def setUp(self):
|
||||
super().setUp()
|
||||
self._reverse_bridge()
|
||||
|
||||
def test_rx_state_not_written_to_trunk_source(self):
|
||||
self._gv(self.trunk, VOICE_HEAD)
|
||||
self.assertEqual(self.trunk.STATUS, {},
|
||||
'RX state must not be written to trunk source STATUS')
|
||||
|
||||
def test_trunk_source_full_call_no_crash(self):
|
||||
"""Full call from trunk source must not raise KeyError."""
|
||||
try:
|
||||
self._gv(self.trunk, VOICE_HEAD)
|
||||
for _ in range(3):
|
||||
self._gv(self.trunk, SLOT1_VOICE)
|
||||
self._gv(self.trunk, VOICE_TERM)
|
||||
except KeyError as exc:
|
||||
self.fail(f'Trunk source group_voice raised KeyError: {exc}')
|
||||
|
||||
|
||||
if __name__ == '__main__':
|
||||
unittest.main()
|
||||
@@ -170,18 +170,15 @@ class TRUNK(asyncio.DatagramProtocol):
|
||||
self._peer_port = trunk_cfg['PEER_PORT']
|
||||
self._peer_sock = trunk_cfg['PEER_SOCK'] # pre-built tuple for sendto()
|
||||
|
||||
# STATUS dict — TS-keyed to match bridgeIPSC.STATUS so the bridge
|
||||
# routing core can write TX state after forwarding to this system.
|
||||
# Contention checks (which read this dict) are bypassed for TRUNK
|
||||
# systems via the TRUNKS list, so only the TX state writes matter.
|
||||
self.STATUS = {
|
||||
1: {'RX_TGID': b'\x00\x00\x00', 'TX_TGID': b'\x00\x00\x00',
|
||||
'RX_TIME': 0, 'TX_TIME': 0,
|
||||
'RX_SRC_SUB': b'\x00\x00\x00', 'TX_SRC_SUB': b'\x00\x00\x00'},
|
||||
2: {'RX_TGID': b'\x00\x00\x00', 'TX_TGID': b'\x00\x00\x00',
|
||||
'RX_TIME': 0, 'TX_TIME': 0,
|
||||
'RX_SRC_SUB': b'\x00\x00\x00', 'TX_SRC_SUB': b'\x00\x00\x00'},
|
||||
}
|
||||
# STATUS is an empty dict. A trunk carries an arbitrary number of
|
||||
# simultaneous streams, so a TS-keyed structure would be a lie.
|
||||
# TX state writes (from the routing core after forwarding TO this
|
||||
# system) are guarded by the TRUNKS check in bridge.py and never
|
||||
# execute. RX state writes (from bridgeTRUNK.group_voice() when
|
||||
# this system is the source) are omitted for the same reason: the
|
||||
# values are only read by contention checks, which are bypassed for
|
||||
# all trunk targets.
|
||||
self.STATUS = {}
|
||||
|
||||
# Outbound stream tracking: (ts, tgid_bytes) -> stream_id_bytes.
|
||||
# Created on VOICE_HEAD (or first burst for late-open); removed on VOICE_TERM.
|
||||
|
||||
Reference in new issue
Block a user