Skip to content

Commit 7021152

Browse files
committed
Merge branch 'main' into feat/retire-aimdb-ws-protocol
2 parents a078f6a + 15d6efd commit 7021152

16 files changed

Lines changed: 440 additions & 868 deletions

File tree

_external/knx-pico

aimdb-knx-connector/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1313

1414
### Fixed
1515

16+
- **Inbound single-octet telegrams no longer decode to `0` (#210).** A telegram carrying exactly one data octet — every DPT5 datapoint (5.001 percentage, 5.003 angle, 5.010 counter, …) — was published as `0` instead of its value. `knx-pico` derived the application data as `[9 .. 7 + npdu_length)`, one octet short of the KNX encoding (the NPDU length octet counts the APCI octet plus the data octets, so the data spans `[9 .. 8 + npdu_length)`); the slice came back empty, the telegram was taken for a 6-bit encoded one, and its value was read out of the APCI octet as `0x80 & 0x3F` — zero. Only single-octet payloads were affected: DPT1 was genuinely 6-bit encoded, and DPT9/DPT14 happened to work because the old code ignored the parsed slice and read to the end of the datagram. Fixed at the root in the fork (`aimdb-dev/knx-pico` `b4883c4`, reported upstream as [cc90202/knx-pico#4](https://github.com/cc90202/knx-pico/issues/4)) — the same off-by-one that made 6-bit telegrams panic, which the previously carried patch had only clamped to an empty slice. `parse_telegram` now reads the parsed frame instead of re-deriving cEMI offsets, so the payload is bounded by the NPDU length octet rather than running to the end of the datagram. **Requires the updated fork** — see the patch note in the [usage guide](../docs/aimdb-usage-guide.md).
1617
- **Heartbeat-response liveness — a dead send path or expired gateway channel now reconnects (review follow-up to #135).** The engine tracks each CONNECTIONSTATE_REQUEST and drops the connection when the gateway's CONNECTIONSTATE_RESPONSE doesn't arrive within the new `TunnelConfig::heartbeat_response_timeout_ms` (default 10 s, the KNX spec timeout) or reports a non-zero status (e.g. the gateway expired the channel during an outage). This restores the old tokio client's recovery from silently-failing sends — the recv path of an unconnected UDP socket never errors, so without it a route flap left the tunnel `Connected` forever with a stale channel id — and adds genuine liveness detection on both runtimes.
1718
- **Pending-ACK tracking is accurate under send failures and bursts.** A frame the transport could not hand to the socket is untracked (`TunnelIo::send` reports success; previously the 3 s sweep warned "ACK timeout" for a telegram that never left the host), and a burst deeper than the 16-entry pending map evicts-and-reports the oldest entry instead of silently dropping its timeout reporting.
1819
- **Tokio: the gateway address is validated in `build()` (issue #133 contract).** A typo'd IP — or a hostname, which `SocketAddr` parsing never resolves — now fails `ConnectorBuilder::build` like on Embassy, instead of producing a healthy-looking connector whose task parked forever logging once per hour.

aimdb-knx-connector/Cargo.toml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[package]
22
name = "aimdb-knx-connector"
3-
version = "0.4.0"
3+
version = "0.5.0"
44
edition = "2021"
55
authors.workspace = true
66
license.workspace = true
@@ -39,8 +39,8 @@ aimdb-core = { version = "1.1.0", path = "../aimdb-core", default-features = fal
3939
aimdb-embassy-adapter = { version = "0.6.0", path = "../aimdb-embassy-adapter", default-features = false, optional = true }
4040

4141
# Use official crates.io version
42-
# External users should patch this to aimdb-dev/knx-pico fork until the
43-
# npdu_length=1 panic fix is upstreamed (DPT9 fix landed upstream in 0.3.0)
42+
# External users should patch this to aimdb-dev/knx-pico fork until the NPDU
43+
# length off-by-one fix is upstreamed (DPT9 fix landed upstream in 0.3.0)
4444
knx-pico = { version = "0.3.0", default-features = false }
4545

4646
# Error handling (std only)

aimdb-knx-connector/src/tunnel.rs

Lines changed: 49 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -729,44 +729,14 @@ fn parse_telegram(cemi_data: &[u8]) -> Option<(GroupAddress, Vec<u8>)> {
729729
// Only process group addresses (not individual addresses)
730730
let dest = ldata.destination_group()?;
731731

732-
// Extract payload (application data)
733-
// For 6-bit encoded values (DPT1 boolean), ldata.data is empty
734-
// and the value is encoded in the APCI byte. We need to extract it manually.
735-
// Note: npdu_length can be 1 (combined TPCI+APCI) or 2 (separate TPCI and APCI)
732+
// An empty `data` slice means the telegram is 6-bit encoded (DPT1 and
733+
// friends): the value rides in the low bits of the APCI octet with no
734+
// data octets following. Everything else carries its value in `data`,
735+
// already bounded by the NPDU length octet.
736736
let payload = if ldata.data.is_empty() {
737-
// 6-bit encoding: extract value from APCI byte in raw cEMI data
738-
// cEMI structure: [msg_code, add_info_len, <add_info>, ctrl1, ctrl2, src(2), dest(2), npdu_len, tpci, apci, ...]
739-
// APCI byte position = 2 + add_info_len + 6 (ctrl1, ctrl2, src(2), dest(2), npdu_len) + 1 (tpci) = 2 + add_info_len + 7 + 1
740-
let add_info_len = if cemi_data.len() > 1 { cemi_data[1] } else { 0 } as usize;
741-
let apci_pos = 2 + add_info_len + 8; // TPCI is at +7, APCI is at +8
742-
743-
if cemi_data.len() > apci_pos {
744-
let apci_byte = cemi_data[apci_pos];
745-
let value = apci_byte & 0x3F; // Extract 6-bit value
746-
vec![value]
747-
} else {
748-
vec![]
749-
}
737+
vec![ldata.six_bit_value()]
750738
} else {
751-
// Standard encoding: multi-byte data (DPT5, DPT7, DPT9, etc.)
752-
//
753-
// cEMI L_Data structure (after msg_code and add_info):
754-
// [0] ctrl1, [1] ctrl2, [2-3] src, [4-5] dest, [6] npdu_len, [7] TPCI, [8] APCI_low, [9+] data
755-
//
756-
// According to knx-pico parser: data starts at position 9 in L_Data
757-
// In full cEMI frame: position = 2 + add_info_len + 9 = 11 (when add_info_len=0)
758-
let add_info_len = if cemi_data.len() > 1 { cemi_data[1] } else { 0 } as usize;
759-
760-
// Data starts at: msg_code(0) + add_info_len_field(1) + add_info(variable) + L_Data_header(9)
761-
let ldata_offset = 2 + add_info_len;
762-
let data_start = ldata_offset + 9; // Position 11 when add_info_len=0
763-
764-
if cemi_data.len() > data_start {
765-
cemi_data[data_start..].to_vec()
766-
} else {
767-
// Fallback to knx-pico's parsed data if extraction fails
768-
ldata.data.to_vec()
769-
}
739+
ldata.data.to_vec()
770740
};
771741

772742
Some((dest, payload))
@@ -971,6 +941,49 @@ mod tests {
971941
);
972942
}
973943

944+
/// A single-octet datapoint (DPT5, e.g. 5.001 humidity in percent) is
945+
/// carried in its own octet after the APCI, not in the APCI's low bits.
946+
/// Reading it as a 6-bit value would silently publish 0.
947+
#[test]
948+
fn inbound_single_octet_telegram_keeps_its_data_byte() {
949+
let addr: GroupAddress = "9/1/1".parse().unwrap();
950+
951+
for raw in [0x7F_u8, 0x80, 0xFF] {
952+
let mut engine = connected_engine(0);
953+
let datagram = inbound_group_write(7, 1, addr, &[raw]);
954+
engine.handle_datagram(&datagram, 100);
955+
let actions = drain(&mut engine);
956+
assert_eq!(
957+
actions[1],
958+
Action::Telegram {
959+
addr,
960+
payload: vec![raw]
961+
},
962+
"raw byte 0x{raw:02X} did not survive the round trip"
963+
);
964+
}
965+
}
966+
967+
/// The NPDU length octet decides how many data octets follow, so a frame
968+
/// with trailing bytes beyond it does not lengthen the payload.
969+
#[test]
970+
fn inbound_telegram_payload_is_bounded_by_npdu_length() {
971+
let addr: GroupAddress = "9/1/0".parse().unwrap();
972+
let mut cemi = build_group_write_cemi(addr, &[0x0C, 0x1A]).to_vec();
973+
cemi.extend_from_slice(&[0xDE, 0xAD]); // trailing octets past the NPDU
974+
975+
let mut engine = connected_engine(0);
976+
engine.handle_datagram(&build_tunneling_request(7, 1, &cemi), 100);
977+
let actions = drain(&mut engine);
978+
assert_eq!(
979+
actions[1],
980+
Action::Telegram {
981+
addr,
982+
payload: vec![0x0C, 0x1A]
983+
}
984+
);
985+
}
986+
974987
#[test]
975988
fn outbound_commands_use_wrapping_sequence_numbers() {
976989
let addr: GroupAddress = "1/0/8".parse().unwrap();

aimdb-knx-connector/tests/frame_building_tests.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,8 @@ mod tests {
6161
let npdu_len = cemi[8] as usize;
6262
assert_eq!(
6363
npdu_len,
64-
2 + data.len(),
65-
"NPDU length should be TPCI + APCI + data"
64+
1 + data.len(),
65+
"NPDU length should be APCI + data"
6666
);
6767
}
6868

@@ -136,7 +136,8 @@ mod tests {
136136
frame.push(0x00);
137137
frame.push(0x80 | (data[0] & 0x3F));
138138
} else {
139-
let npdu_len = 2 + data.len();
139+
// NPDU length counts the APCI octet plus the data octets.
140+
let npdu_len = 1 + data.len();
140141
frame.push(npdu_len as u8);
141142
frame.push(0x00);
142143
frame.push(0x80);

aimdb-sync/CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1414
### Changed (breaking)
1515

1616
- **Issue #131:** `AimDbSyncExt` extends the non-generic `aimdb_core::AimDb`; internal handles drop the `TokioAdapter` type parameter.
17+
- **Issue #200:** the internal channel bridge to the `tokio` thread is gone — blocking calls now call the runtime directly using the `block_on` seam. API implications:
18+
- `SyncProducer::set_with_timeout` removed
19+
- Capacity-related API removed: `AimDbBuilderSyncExt::producer_with_capacity`/`consumer_with_capacity` and the `DEFAULT_SYNC_CHANNEL_CAPACITY` constant are gone.
20+
- `SyncConsumer`: `get`, `try_get`, `get_with_timeout`, `get_latest`, and `get_latest_with_timeout` now take `&mut self` (was `&self`)
21+
- `SyncConsumer` no longer implements `Clone` or `Sync` (still `Send`).
1722

1823
### Changed
1924

0 commit comments

Comments
 (0)