feat: foreign device registration

[EveGun]

Summary

This PR adds BACnet Foreign Device Registration (FDR) support and BBMD-based Who-Is discovery to @bacnet-js/client, including forwarded response handling and examples for manual validation.

Fixes #54

What changed

• Added registerForeignDevice(receiver, ttl) to BACnetClient:
  • Sends BVLC REGISTER_FOREIGN_DEVICE (0x05)
  • Resolves/rejects based on BVLC_RESULT from the target BBMD
  • Validates TTL range (1..65535)
  • Requires BBMD address format host:port
  • Serializes parallel registrations per BBMD target to avoid ambiguous result correlation
• Added whoIsThroughBBMD(bbmd, options?) to BACnetClient:
  • Sends Who-Is wrapped in BVLC DISTRIBUTE_BROADCAST_TO_NETWORK (0x09)
• Extended outbound BVLC encode path:
  • Added support for distributeBroadcastToNetwork receiver flag in sendBvlc
• Extended inbound BVLC handling:
  • Added BVLC_RESULT parsing/emission via bvlcResult event
  • Added guard to require BVLC-Result payload length before decode
  • Preserved FORWARDED_NPDU origin metadata (forwardedFrom)
• Improved decode robustness:
  • Added guard for invalid BVLC header length (msgLength < BVLC_HEADER_LENGTH)
  • IAm.decode now reports correct decoded len
• Added examples:
  • examples/register-foreign-device.ts
  • examples/discover-devices-via-bbmd.ts
• Added/extended tests:
  • Unit coverage for FDR success/NAK/timeout/address validation
  • Unit coverage for dedupe + serialized handling of parallel registrations
  • Unit coverage for queued retry behavior after failed prior registration
  • Unit coverage for BBMD Who-Is (0x09)
  • BVLC strict-length behavior test (reject trailing bytes)
  • IAm decode-length assertion
  • Improved register-foreign-device integration payload assertions

Type consistency

• Added BACnetClientEvents.bvlcResult typing
• Added BvlcResultPayload type
• Added BACNetAddress.distributeBroadcastToNetwork?: boolean

Validation

• node --require esbuild-register --test test/unit/client.spec.ts test/unit/bvlc.spec.ts test/unit/service-i-am.spec.ts
• npm run test:unit mostly green locally; one flaky test (request-manager.spec.ts) failed once and passed on isolated rerun
• npm run test:all remains environment-dependent for integration/compliance tests

Notes

• FDR lifecycle policy (renew/unregister) remains caller-controlled by design
• BBMD support here targets discovery/FDR paths; general BACnet unicast routing remains topology/network-policy dependent
• Trailing-byte BVLC tolerance was intentionally not included in this PR to keep scope focused and avoid cross-cutting parser-policy changes

[EveGun]

@robertsLando

I’ve implemented BBMD/FDR support in this PR and validated it in my environment, but I can only test against a single BBMD setup.

Could you or some other maintainers help with broader validation, especially across multiple BBMD/network topologies?

I can’t fully cover integration/compliance scenarios without a richer BACnet setup.

[Copilot]

Pull request overview

Adds BACnet Foreign Device Registration (FDR) and BBMD-based Who-Is discovery support to @bacnet-js/client, extending BVLC encode/decode and event typing to handle BVLC-Result responses and BBMD-forwarded metadata.

Changes:

• Added BACnetClient.registerForeignDevice() with BVLC-Result correlation/timeout handling and parallel-call serialization.
• Added BACnetClient.whoIsThroughBBMD() plus BVLC outbound support for Distribute-Broadcast-To-Network (0x09).
• Improved BVLC/service decode behavior and expanded unit/integration coverage + examples for manual validation.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/lib/client.ts	Adds FDR + BBMD Who-Is APIs, BVLC-Result event emission, and outbound BVLC 0x09 support.
src/lib/bvlc.ts	Adjusts BVLC decode to tolerate trailing UDP bytes and adds FORWARDED_NPDU length guard.
src/lib/services/IAm.ts	Fixes decoded len reporting.
src/lib/types.ts	Extends address type with distributeBroadcastToNetwork and adds BvlcResultPayload.
src/lib/EventTypes.ts	Adds typed bvlcResult event.
test/unit/client.spec.ts	Adds unit tests for FDR success/NAK/timeout/validation + BBMD Who-Is behavior.
test/unit/bvlc.spec.ts	Adds unit test for BVLC decode with trailing UDP bytes.
test/unit/service-i-am.spec.ts	Asserts IAm.decode() reports correct len.
test/integration/register-foreign-device.spec.ts	Tightens encode/decode assertions for FDR payload TTL.
examples/register-foreign-device.ts	Example script to register/renew FDR against a BBMD.
examples/discover-devices-via-bbmd.ts	Example workflow: FDR then Who-Is via BBMD (BVLC 0x09).

Comments suppressed due to low confidence (2)

src/lib/client.ts:819

• BVLC decoding now tolerates trailing UDP bytes via result.msgLength, but _receiveData still uses buffer.length - result.len as the NPDU length. This will cause trailing bytes to be parsed as part of the NPDU/APDU instead of being ignored. Use result.msgLength - result.len (and similarly for other branches) when passing lengths into _handleNpdu/service decoders.

			case BvlcResultPurpose.ORIGINAL_UNICAST_NPDU:
			case BvlcResultPurpose.ORIGINAL_BROADCAST_NPDU:
				this._handleNpdu(
					buffer,
					result.len,
					buffer.length - result.len,
					header,
				)

src/lib/client.ts:2409

• When receiver.distributeBroadcastToNetwork is set, sendBvlc() encodes BVLC 0x09 but still calls _send() with receiver?.address. If address is missing, Transport.send() will broadcast the packet, which is not valid for Distribute-Broadcast-To-Network (it should be unicasted to a BBMD). Add a validation that receiver.address is present (or fall back to unicast semantics) when distributeBroadcastToNetwork is true.

		} else if (receiver && receiver.distributeBroadcastToNetwork) {
			// Foreign device broadcast distribution through BBMD (BVLC 0x09)
			baBvlc.encode(
				buffer.buffer,
				BvlcResultPurpose.DISTRIBUTE_BROADCAST_TO_NETWORK,
				buffer.offset,
			)
		} else if (receiver && receiver.address) {
			// Specific address, unicast
			baBvlc.encode(
				buffer.buffer,
				BvlcResultPurpose.ORIGINAL_UNICAST_NPDU,
				buffer.offset,
			)
		} else {
			// No address, broadcast
			baBvlc.encode(
				buffer.buffer,
				BvlcResultPurpose.ORIGINAL_BROADCAST_NPDU,
				buffer.offset,
			)
		}

		this._send(buffer, receiver)
	}

[robertsLando]

@robertsLando

I’ve implemented BBMD/FDR support in this PR and validated it in my environment, but I can only test against a single BBMD setup.

Could you or some other maintainers help with broader validation, especially across multiple BBMD/network topologies?

I can’t fully cover integration/compliance scenarios without a richer BACnet setup.

Actually I don't have anyone that could test this in the near future, dunno if @jacoscaz can

[jacoscaz]

@robertsLando
I’ve implemented BBMD/FDR support in this PR and validated it in my environment, but I can only test against a single BBMD setup.
Could you or some other maintainers help with broader validation, especially across multiple BBMD/network topologies?
I can’t fully cover integration/compliance scenarios without a richer BACnet setup.

Actually I don't have anyone that could test this in the near future, dunno if @jacoscaz can

Right now I don't have access to anything of the sort, unfortunately. I might gain access to a much richer BACnet setup towards the end of march; if this materialises I'd likely be able to "book" it for a brief testing session.

[EveGun]

Converted to draft while trying to get a better testing setup. Will open again later.

[EveGun]

Removed the trailing-bytes BVLC change from this PR. While interoperability can benefit from tolerance, it affects multiple decode paths and requires a consistent msgLength-bounded parsing policy across the stack(thanks to Copilot for noticing). To keep this PR focused and low-risk around FDR/BBMD behavior, I kept strict BVLC length validation

[EveGun]

@robertsLando

I’ve spent the day testing this as thoroughly as possible in my simple lab setup across multiple subnets. Since I only have access to a single BBMD device (I have several, but they all are Wago's), I can not validate this any further.

Based on my testing, the changes seem to have a low impact and I don't see a high risk for regressions in other parts of the code, after I removed the trailing-bytes opening. I’ll leave it up to you to decide if you want to pull this in now based on these results, or if you'd rather wait for a test in a more complex environment. 🤞

[robertsLando]

Thanks for this PR — solid work overall. The protocol compliance with ASHRAE 135 Annex J looks correct (Register-Foreign-Device 0x05, BVLC-Result 0x00, Distribute-Broadcast-To-Network 0x09 all match the spec). The BVLC decode guards, serialization logic, and test coverage are well done.

A few issues to address before merging:

Issues

1. whoIs overload detection regression (client.ts:899)

The if (!options) guard was removed from the overload sniffing. Original code skipped sniffing when options was explicitly provided as the 2nd arg. The new code always sniffs receiverOrOptions regardless, which means if arg1 is a BACNetAddress that happens to have a lowLimit/highLimit property (duck typing), the explicitly-passed options gets silently overwritten. The guard should be restored.

2. Wasteful buffer allocation before dedup check (client.ts:1002)

_getApduBuffer() (allocates 1482 bytes) and RegisterForeignDevice.encode() run before checking the pending registration map at line 1018. If the dedup path is taken (pending.ttl === ttl), the buffer is thrown away unused. Same for the serialization path (different TTL) — the buffer is discarded since the recursive call allocates a new one. Move the buffer allocation + encoding after the pending check (just before the new Promise at line 1032).

3. DEFAULT_BACNET_PORT duplication (client.ts:127, bvlc.ts:8)

The constant 47808 is now defined in both client.ts and bvlc.ts. Should be imported from a shared location (e.g., enum.ts) to avoid drift.

4. Timeout not unref()'d (client.ts:1033)

The setTimeout inside registerForeignDevice will keep the Node.js event loop alive. If this is the last pending operation, the process won't exit cleanly until the timeout fires. Consider timeout.unref().

Minor / Non-blocking

• IAm.decode len fix (IAm.ts:77): Correct fix but the expression apduLen + (offset - orgOffset) is misleading — offset is never mutated so (offset - orgOffset) is always 0. Could be simplified to just apduLen.
• _normalizeAddress inconsistency: "127.0.0.1" (no colon) appends default port → "127.0.0.1:47808", but "127.0.0.1:" (trailing colon) returns null. Both semantically mean "default port" but are handled differently.
• BVLC-Result scope: Result matching is by sender address only, not by originating request type. Fine today since only Register-FD awaits results, but worth a comment noting this limitation if other BVLC operations are added later.

[robertsLando]

Additional findings from deeper review

5. Wrong NPDU destination for Distribute-Broadcast-To-Network (HIGH)

When whoIsThroughBBMD() calls whoIs(), the NPDU layer encodes the BBMD address as the network-layer destination:

baNpdu.encode(buffer, NpduControlPriority.NORMAL_MESSAGE, receiver, ...)

Per ASHRAE 135 Annex J.11, Distribute-Broadcast-To-Network (0x09) tells the BBMD to broadcast the enclosed NPDU to its local network. The BBMD is only the BVLC-level UDP target — the NPDU destination should indicate broadcast (omitted or net=0xFFFF), not the BBMD's unicast address. Spec-compliant BBMDs could misroute or reject this packet.

Fix: When distributeBroadcastToNetwork is true, pass undefined as the NPDU destination so it encodes as a local broadcast. Only use the BBMD address for the UDP _send() target.

6. No cleanup of pending FDR registrations on close()

If client.close() is called while registerForeignDevice() is in-flight:

• The setTimeout keeps the event loop alive until apduTimeout fires
• The bvlcResult listener stays attached to the closed client
• The caller's promise hangs until timeout

close() should reject all pending registrations and clear the _pendingForeignDeviceRegistrations map.

7. BVLC-Result success code is ambiguous across operations

Per ASHRAE 135 Annex J.2.1, result code 0x0000 (Successful Completion) is shared by all BVLC operations. The onResult handler resolves the FDR promise on any 0x0000 from the matching BBMD. If the BBMD sends a success result for a different operation (e.g., Write-BDT), the FDR would incorrectly resolve.

NAK codes do encode the operation type (0x0030 = Register-FD NAK, 0x0060 = Distribute-Broadcast NAK), so failures are correctly distinguished. But success is not. Worth a comment noting this inherent protocol limitation.

8. Unbounded recursion in different-TTL serialization

The serialization path for different-TTL requests to the same BBMD recurses:

await pending.promise
return this.registerForeignDevice(receiver, ttl)  // recursive call

If N callers queue with different TTLs, this creates N-deep recursion. Consider a loop or iterative approach instead.

Minor

• Example type violation (examples/register-foreign-device.ts): Passes null for optional fields (net: null, adr: null, forwardedFrom: null) but BACNetAddress declares them as net?: number — optional, not nullable. The discover example correctly uses just { address: bbmdAddress }.
• distributeBroadcastToNetwork on BACNetAddress: Adds transport-level behavior ("how to send") to an address type ("where to send"). A BACNetAddress with this flag could accidentally propagate to methods that don't expect it (e.g., stored in a device table and later passed to readProperty). Consider a separate options param or wrapper type.

[robertsLando]

@EveGun I improved tests to make them less flaky

[robertsLando]

All 8 issues from the previous reviews have been properly addressed. Nice work on the new commits.

Verification

#	Issue	Status
1	whoIs overload guard	✅ if (!options) restored
2	Buffer before dedup	✅ Moved after while loop
3	DEFAULT_BACNET_PORT dup	✅ Centralized in enum.ts
4	setTimeout not unref()'d	✅ Added with typeof guard
5	Wrong NPDU dest for 0x09	✅ npduDestination = undefined for DBTN
6	No cleanup on close()	✅ Rejects pending + _isClosed flag
7	BVLC success ambiguity	✅ Comment documenting limitation
8	Unbounded recursion	✅ Replaced with while (true) loop

The while loop replacing recursion, _isClosed with close() cleanup, settled guard, and integration test improvements (assert.rejects + t.after) are all well done.

Minor (non-blocking)

See inline comment about the redundant rejectRegistration assignment.