Dev by jscott3201 · Pull Request #1 · jscott3201/rusty-bacnet

jscott3201 · 2026-03-06T23:01:12Z

CLI Updates

Copilot

Pull request overview

This PR bumps the version from 0.6.1 to 0.6.2 and adds new CLI capabilities for cross-subnet and directed BACnet device discovery. It introduces three new discover variants: directed unicast (--target), network-specific broadcast (--dnet), and BBMD-assisted discovery (--bbmd). The functionality is plumbed through a new broadcast_to_network in the network layer, who_is_directed and who_is_network in the BACnet client, and new discover_directed/discover_network command implementations. The dev CI branch is also added to the GitHub Actions triggers.

Changes:

New discover modes: --target, --dnet, and --bbmd flags added to the CLI discover command (both one-shot and interactive shell)
New network/client API: NetworkLayer::broadcast_to_network, BACnetClient::who_is_directed, and BACnetClient::who_is_network
Version bump from 0.6.1 to 0.6.2 across all Rust crates, Java packaging, and documentation

Reviewed changes

Copilot reviewed 13 out of 14 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
`Cargo.toml`	Version bump 0.6.1 → 0.6.2 for workspace and all workspace dependencies
`Cargo.lock`	Lockfile regenerated for the version bump
`benchmarks/Cargo.toml`	Version bump
`java/gradle.properties`	Java/Kotlin binding version bump
`README.md`	Dependency snippet updated to 0.6.2
`examples/kotlin/README.md`	JAR filename references updated
`examples/kotlin/BipClientServer.kt`	Usage comment JAR filename updated
`CHANGELOG.md`	New 0.6.2 section documenting all additions and fixes
`.github/workflows/ci.yml`	Added `dev` branch to CI push/PR triggers
`crates/bacnet-network/src/layer.rs`	New `broadcast_to_network` method for NPDU with specific DNET
`crates/bacnet-client/src/client.rs`	New `who_is_directed` and `who_is_network` public API methods
`crates/bacnet-cli/src/commands/discover.rs`	New `discover_directed` and `discover_network` command functions
`crates/bacnet-cli/src/shell.rs`	`--target`/`--dnet` parsing in interactive shell; rustyline `PreferTerm` behavior
`crates/bacnet-cli/src/main.rs`	New CLI flags; `parse_discover_range` refactor; BBMD integration in `execute_bip_command`

Copilot · 2026-03-06T23:07:35Z

crates/bacnet-network/src/layer.rs

+    pub async fn broadcast_to_network(
+        &self,
+        apdu: &[u8],
+        dest_network: u16,
+        expecting_reply: bool,
+        priority: NetworkPriority,
+    ) -> Result<(), Error> {
+        let npdu = Npdu {
+            is_network_message: false,
+            expecting_reply,
+            priority,
+            destination: Some(NpduAddress {
+                network: dest_network,
+                mac_address: MacAddr::new(),
+            }),
+            source: None,
+            hop_count: 255,
+            payload: Bytes::copy_from_slice(apdu),
+            ..Npdu::default()
+        };
+
+        let mut buf = BytesMut::with_capacity(8 + apdu.len());
+        encode_npdu(&mut buf, &npdu)?;
+        self.transport.send_broadcast(&buf).await


The test suite in layer.rs includes global_broadcast_npdu_has_dnet_ffff (verifying DNET=0xFFFF encoding) and routed_send_encodes_dnet_dadr (verifying routed unicast encoding), but there is no equivalent test verifying that broadcast_to_network correctly encodes the NPDU with the specified dest_network value. Given that the parallel test for broadcast_global_apdu exists, a test like broadcast_to_network_encodes_specific_dnet should be added to confirm the NPDU destination network is encoded correctly.

Copilot · 2026-03-06T23:07:36Z

crates/bacnet-cli/src/main.rs

+                let mac = resolve::parse_target(target_str)
+                    .and_then(|t| match t {
+                        resolve::Target::Mac(m) => Ok(m),
+                        _ => Err("--target requires an IP address, not a device instance".into()),


The error message "--target requires an IP address, not a device instance" will also be displayed when a routed address (e.g., 2:1234) is passed. The message is inaccurate for that case since a routed address is not a "device instance". A more accurate message would be "--target requires an IP address (e.g., 192.168.1.10), not a device instance number or routed address".

Suggested change

_ => Err("--target requires an IP address, not a device instance".into()),

_ => Err(

"--target requires an IP address (e.g., 192.168.1.10), not a device instance number or routed address"

.into(),

),

Copilot · 2026-03-06T23:07:36Z

crates/bacnet-cli/src/main.rs

+                let mac = resolve::parse_target(target_str)
+                    .and_then(|t| match t {
+                        resolve::Target::Mac(m) => Ok(m),
+                        _ => Err("--target requires an IP address, not a device instance".into()),


The error message "--target requires an IP address, not a device instance" will also be shown when a routed address (e.g., 2:1234) is passed as --target. The message is inaccurate for the routed address case.

Copilot · 2026-03-06T23:07:36Z

crates/bacnet-cli/src/shell.rs

+                    .await
+            }
+            Ok(_) => {
+                output::print_error("--target requires an IP address, not a device instance");


The error message "--target requires an IP address, not a device instance" will also be shown when a routed address (e.g., 2:1234) is passed as --target. The message is inaccurate for the routed address case.

Suggested change

output::print_error("--target requires an IP address, not a device instance");

output::print_error(

"--target requires a direct IP/MAC address, not a device instance or routed address",

);

Copilot · 2026-03-06T23:07:36Z

crates/bacnet-cli/src/main.rs

+        /// Send directed WhoIs to a specific address instead of broadcasting.
+        #[arg(long)]
+        target: Option<String>,
+        /// Register as foreign device with a BBMD before discovering.
+        #[arg(long)]
+        bbmd: Option<String>,
+        /// TTL in seconds for BBMD foreign device registration.
+        #[arg(long, default_value_t = 300)]
+        ttl: u16,
+        /// Target a specific remote network number.
+        #[arg(long)]
+        dnet: Option<u16>,


The --target and --dnet flags are not declared as mutually exclusive in the clap argument definition. When both are provided, --target silently takes precedence and --dnet is ignored without any warning to the user. Adding #[arg(long, conflicts_with = "dnet")] to the target field (or conflicts_with = "target" to dnet) would cause clap to report an error at argument parsing time, preventing silent behavior that could be confusing.

Copilot · 2026-03-06T23:07:36Z

crates/bacnet-client/src/client.rs

+        let pdu = Apdu::UnconfirmedRequest(bacnet_encoding::apdu::UnconfirmedRequest {
+            service_choice: UnconfirmedServiceChoice::WHO_IS,
+            service_request: Bytes::copy_from_slice(&buf),
+        });
+
+        let mut apdu_buf = BytesMut::with_capacity(2 + buf.len());
+        encode_apdu(&mut apdu_buf, &pdu);
+
+        self.network
+            .send_apdu(&apdu_buf, destination_mac, false, NetworkPriority::NORMAL)


The who_is_directed method manually encodes the APDU by constructing Apdu::UnconfirmedRequest and calling encode_apdu, then passing the result to network.send_apdu. This duplicates the logic already available in self.unconfirmed_request(destination_mac, UnconfirmedServiceChoice::WHO_IS, &buf), which performs exactly the same steps. The existing who_is uses broadcast_global_unconfirmed as its helper for the same reason. Using the existing helper would make this consistent, less error-prone, and easier to maintain.

Suggested change

let pdu = Apdu::UnconfirmedRequest(bacnet_encoding::apdu::UnconfirmedRequest {

service_choice: UnconfirmedServiceChoice::WHO_IS,

service_request: Bytes::copy_from_slice(&buf),

});

let mut apdu_buf = BytesMut::with_capacity(2 + buf.len());

encode_apdu(&mut apdu_buf, &pdu);

self.network

.send_apdu(&apdu_buf, destination_mac, false, NetworkPriority::NORMAL)

self

.unconfirmed_request(destination_mac, UnconfirmedServiceChoice::WHO_IS, &buf)

Copilot · 2026-03-06T23:07:37Z

crates/bacnet-client/src/client.rs

+        let pdu = Apdu::UnconfirmedRequest(bacnet_encoding::apdu::UnconfirmedRequest {
+            service_choice: UnconfirmedServiceChoice::WHO_IS,
+            service_request: Bytes::copy_from_slice(&buf),
+        });
+
+        let mut apdu_buf = BytesMut::with_capacity(2 + buf.len());
+        encode_apdu(&mut apdu_buf, &pdu);
+
+        self.network
+            .broadcast_to_network(&apdu_buf, dest_network, false, NetworkPriority::NORMAL)
+            .await


The who_is_network method manually encodes the APDU (constructing Apdu::UnconfirmedRequest, calling encode_apdu) before passing it to network.broadcast_to_network. This duplicates the APDU-encoding logic already present in broadcast_global_unconfirmed and unconfirmed_request. Consider adding a broadcast_network_unconfirmed(service_choice, service_data, dest_network) helper on the client, so that who_is_network can follow the same pattern as who_is (which delegates to broadcast_global_unconfirmed). This would keep all three WhoIs variants consistent.

Copilot · 2026-03-06T23:07:37Z

crates/bacnet-network/src/layer.rs

+    pub async fn broadcast_to_network(
+        &self,
+        apdu: &[u8],
+        dest_network: u16,
+        expecting_reply: bool,
+        priority: NetworkPriority,
+    ) -> Result<(), Error> {
+        let npdu = Npdu {
+            is_network_message: false,
+            expecting_reply,
+            priority,
+            destination: Some(NpduAddress {
+                network: dest_network,
+                mac_address: MacAddr::new(),
+            }),
+            source: None,
+            hop_count: 255,
+            payload: Bytes::copy_from_slice(apdu),
+            ..Npdu::default()
+        };
+
+        let mut buf = BytesMut::with_capacity(8 + apdu.len());
+        encode_npdu(&mut buf, &npdu)?;
+        self.transport.send_broadcast(&buf).await


The broadcast_to_network function does not validate that dest_network is not 0xFFFF (65535). In BACnet, DNET=0xFFFF is the reserved value meaning "all networks" — it is semantically equivalent to calling broadcast_global_apdu. If a user passes --dnet 65535, the packet produced would be identical to a global WhoIs broadcast, silently defeating the intent of broadcast_to_network. Adding an early check like if dest_network == 0xFFFF { return Err(Error::Encoding("dest_network 0xFFFF is reserved for global broadcasts; use broadcast_global_apdu instead".into())) } would make the API safer and clearer.

jscott3201 added 2 commits March 6, 2026 17:51

Version 0.6.2 - See changelog for updates.

1037567

Version 0.6.2 - See changelog for updates.

08181f5

jscott3201 self-assigned this Mar 6, 2026

Copilot AI review requested due to automatic review settings March 6, 2026 23:01

Copilot started reviewing on behalf of jscott3201 March 6, 2026 23:01 View session

jscott3201 merged commit 691e41f into main Mar 6, 2026
38 checks passed

Copilot AI reviewed Mar 6, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Dev#1

Dev#1
jscott3201 merged 2 commits intomainfrom
dev

jscott3201 commented Mar 6, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 6, 2026

Uh oh!

Copilot AI Mar 6, 2026

Uh oh!

Copilot AI Mar 6, 2026

Uh oh!

Copilot AI Mar 6, 2026

Uh oh!

Copilot AI Mar 6, 2026

Uh oh!

Copilot AI Mar 6, 2026

Uh oh!

Copilot AI Mar 6, 2026

Uh oh!

Copilot AI Mar 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

jscott3201 commented Mar 6, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants