Skip to content

Per-property reads encode the raw max-APDU octet count into a 4-bit enum field, producing wrong or reserved values #55

Description

@ravz

Summary

The per-property fallback reads in _readObjectFull and _readObjectLite pass the device's raw max-APDU octet count into the APDU max-APDU-length-accepted field, which is a 4-bit enum (0–5). The value is truncated and OR-ed into a single byte, so the request tells the device we accept a wildly wrong APDU size — sometimes a reserved value that isn't valid BACnet at all.

This is the path that reads OBJECT_NAME per object, and objects with no OBJECT_NAME are dropped from the model — the [discovery] dropped <type>:<instance> on device <id> — no OBJECT_NAME returned (read likely rejected) log. Surfaced while investigating #49.

Detail

bacnet_client.js:1447-1450 (_readObjectFull) and bacnet_client.js:1578-1581 (_readObjectLite):

const readIndividualPropsOptions = {
  maxSegments: 0,
  maxApdu: device.getMaxApdu(),   // <-- raw octet count from the I-Am (e.g. 480), not the 0-5 enum
};

Compare getDeviceSpecificOptions (bacnet_client.js:1046-1075), which correctly maps octets to the enum (<=50 -> 0, <=128 -> 1, <=206 -> 2, <=480 -> 3, <=1024 -> 4, else 5).

The encoder writes both fields into one byte with no masking — apdu.js:

buffer.buffer[buffer.offset++] = maxSegments | maxApdu;

so a raw octet count overflows into the max-segments bits and truncates to & 0xFF.

Verified

Capturing the emitted APDU and decoding byte 7 (max-segments | max-APDU):

what the device is TOLD we accept:

correct (enum)  maxApdu: 5         byte=0x75 -> max-segments=7 max-APDU=5 (1476 octets)
correct (enum)  maxApdu: 3         byte=0x73 -> max-segments=7 max-APDU=3 (480 octets)

RAW octets      maxApdu: 1476      byte=0xf4 -> max-segments=7 max-APDU=4 (1024 octets)
RAW octets      maxApdu: 1024      byte=0x70 -> max-segments=7 max-APDU=0 (50 octets)
RAW octets      maxApdu: 480       byte=0xf0 -> max-segments=7 max-APDU=0 (50 octets)
RAW octets      maxApdu: 206       byte=0xfe -> max-segments=7 max-APDU=14 (reserved/invalid)
RAW octets      maxApdu: 128       byte=0xf0 -> max-segments=7 max-APDU=0 (50 octets)
RAW octets      maxApdu: 50        byte=0x72 -> max-segments=7 max-APDU=2 (206 octets)

Three distinct failures:

  1. A 480-octet device is told we accept 50 octets. Any OBJECT_NAME response longer than 50 octets cannot be returned unsegmented, and this request also declares no segmentation intent worth honouring — so the device rejects or drops it.
  2. A 206-octet device produces max-APDU 14, a reserved value. That is a malformed request; conforming devices are entitled to reject it outright.
  3. maxSegments: 0 never survives. The raw octet value's high bits bleed into the max-segments field, so every one of these requests advertises "more than 64 segments accepted" regardless of what the caller asked for.

Case 3 also means the explicit intent of that options object — "no segmentation, small device" — is silently inverted on the wire.

Proposed fix

Use the existing octets→enum mapping instead of the raw value. Extract the mapping out of getDeviceSpecificOptions into a helper (e.g. encodeMaxApdu(octets)) and call it from all three sites, so the conversion exists in exactly one place.

Defensively, encodeConfirmedServiceRequest should mask its inputs ((maxSegments & 0x70) | (maxApdu & 0x0F)) so an out-of-range value can never corrupt the adjacent field silently.

Acceptance criteria

  1. For every device max-APDU (50, 128, 206, 480, 1024, 1476), the individual-property read encodes the matching enum, and the decoded octet capacity is never larger than what the device advertised.
  2. maxSegments: 0 encodes as max-segments 0, not 7.
  3. No input produces a reserved max-APDU value.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions