5
3
Fork
You've already forked bmputil
2

Feature: BMD RSP v0 support #82

Merged
dragonmux merged 49 commits from feature/full-bmp-rsp-support into main 2026年05月07日 08:52:19 +02:00

In this PR we sort out power control and fully implement BMD remote protocol v0. This sees all the major components for remote protocol support fleshed out better - including the protocol traits so they return more suitable types, better handling of responses via a Response type that wraps a load of the decode and dispatch up neatly, and a bunch of other helper types such as one for handling u32's that also need to maintain their parity information, and another for properly handling ADI register encoding.

This also provides a test suite for part of the new remote protocol code to try and ensure that at least parts of this are known functional.

The branch this is on will be used for additional remote protocol development as this is the first of a series of 4 or 5 PRs implementing full remote protocol support into bmputil, so should be left after merge.

In this PR we sort out power control and fully implement BMD remote protocol v0. This sees all the major components for remote protocol support fleshed out better - including the protocol traits so they return more suitable types, better handling of responses via a Response type that wraps a load of the decode and dispatch up neatly, and a bunch of other helper types such as one for handling u32's that also need to maintain their parity information, and another for properly handling ADI register encoding. This also provides a test suite for part of the new remote protocol code to try and ensure that at least parts of this are known functional. The branch this is on will be used for additional remote protocol development as this is the first of a series of 4 or 5 PRs implementing full remote protocol support into bmputil, so should be left after merge.
This will need refinement as the handling of parity errors is not right yet.
SWD never has a transaction longer than 32 (+1) bits, so that's fine as the +1 is always the parity bit, which the parity variants deal with
serial/remote/response: Document some of the Response type API
All checks were successful
ci/woodpecker/pr/build-windows/2 Pipeline was successful
ci/woodpecker/pr/build-linux Pipeline was successful
ci/woodpecker/pr/build-windows/1 Pipeline was successful
67a8614f13
dragonmux force-pushed feature/full-bmp-rsp-support from 67a8614f13
All checks were successful
ci/woodpecker/pr/build-windows/2 Pipeline was successful
ci/woodpecker/pr/build-linux Pipeline was successful
ci/woodpecker/pr/build-windows/1 Pipeline was successful
to 6d34334c31
All checks were successful
ci/woodpecker/pr/build-windows/2 Pipeline was successful
ci/woodpecker/pr/build-linux Pipeline was successful
ci/woodpecker/pr/build-windows/1 Pipeline was successful
2026年05月01日 03:12:34 +02:00 Compare
serial/remote/response: Cleaned up the API for Response, making the functions more Rust-ily named
All checks were successful
ci/woodpecker/pr/build-windows/2 Pipeline was successful
ci/woodpecker/pr/build-linux Pipeline was successful
ci/woodpecker/pr/build-windows/1 Pipeline was successful
dc72e0eff1
northernpaws left a comment
Copy link

Looks good to me, but a couple of things that need to be changed. See notes below.

Looks good to me, but a couple of things that need to be changed. See notes below.
@ -61,0 +63,4 @@
Align::As8Bit=>0,
Align::As16Bit=>1,
Align::As32Bit=>2,
Align::As64Bit=>4,

1 << 4 is 16 not 8, address is over aligned.

`1 << 4` is 16 not 8, address is over aligned.
Author
Owner
Copy link

Good catch - switched to 1 << 3 so we only align to the nearest 8 bytes.

Good catch - switched to `1 << 3` so we only align to the nearest 8 bytes.
dragonmux marked this conversation as resolved
@ -377,0 +638,4 @@
Err(eyre!("{operation}: Unexpected error {error}"))
},
// Check if the remote is reporting a parameter error
ResultCode::ParameterError=>Err(eyre!("{operation}: !BUG! Firmware reported a parameter error\n")),

Newline not required here.

Newline not required here.
dragonmux marked this conversation as resolved
dragonmux force-pushed feature/full-bmp-rsp-support from dc72e0eff1
All checks were successful
ci/woodpecker/pr/build-windows/2 Pipeline was successful
ci/woodpecker/pr/build-linux Pipeline was successful
ci/woodpecker/pr/build-windows/1 Pipeline was successful
to d104cefa6b
All checks were successful
ci/woodpecker/pr/build-windows/2 Pipeline was successful
ci/woodpecker/pr/build-linux Pipeline was successful
ci/woodpecker/pr/build-windows/1 Pipeline was successful
2026年05月04日 07:27:33 +02:00 Compare
freyjadomville left a comment
Copy link

Nitpicks.

Nitpicks.
@ -550,3 +558,3 @@
letremote=device.bmd_serial_interface()?.remote()?;
letpower=remote.get_target_power_state()?;
matchpower_args.supply_power{

Less indentation needed if this is an if let Some(state) = power_args.supply_power { /* ... */ } else { /* ... */ }.

Similar potential for any two-case matches in this PR (enable/disable, etc.).

Less indentation needed if this is an `if let Some(state) = power_args.supply_power { /* ... */ } else { /* ... */ }`. Similar potential for any two-case matches in this PR (enable/disable, etc.).
Author
Owner
Copy link

Good point, that's fixed and intent comments added.

Good point, that's fixed and intent comments added.
dragonmux marked this conversation as resolved
@ -160,16 +175,16 @@ impl BmdRspInterface
// If this was because of REMOTE_EOM, return
ifbuffer[offset]==REMOTE_EOM{
buffer[offset]=0;

Why is this safe? Is the reason the same as above (UTF-8)?

Why is this safe? Is the reason the same as above (UTF-8)?
Author
Owner
Copy link

NUL character replacement of a single byte character that is in basic ASCII cannot ever produce invalid UTF-8 - look at how REMOTE_EOM is defined. It's intrinsically safe because of that definition.

Edit: Looks like Codeberg is displaying your notes in an odd position - above rather than on the line you were asking about, very confusingly.. so we thought you were asking about buffer[offset] = 0; rather than the unsafe following it.

NUL character replacement of a single byte character that is in basic ASCII cannot ever produce invalid UTF-8 - look at how REMOTE_EOM is defined. It's intrinsically safe because of that definition. Edit: Looks like Codeberg is displaying your notes in an odd position - above rather than on the line you were asking about, very confusingly.. so we thought you were asking about `buffer[offset] = 0;` rather than the `unsafe` following it.
freyjadomville marked this conversation as resolved
@ -0,0 +55,4 @@
self.as_str().unwrap_or("unknown")
}
pubfn as_string(&self)-> &str

Worth documenting that this is safe because of ::new()

Worth documenting that this is safe because of ::new()
freyjadomville marked this conversation as resolved
@ -0,0 +223,4 @@
Self::ParameterError=>write!(fmt,"P"),
Self::Error=>write!(fmt,"E"),
Self::NotSupported=>write!(fmt,"N"),
Self::Empty=>write!(fmt,"<empty>"),

You could match data here and then if let inside the match if you wanted to reduce nesting - https://github.com/rust-lang/rust/pull/141295

You *could* match data here and then `if let` inside the match if you wanted to reduce nesting - https://github.com/rust-lang/rust/pull/141295
Author
Owner
Copy link

Unclear how that'd work in this case so if you could include a worked example it would be useful. This also appears to be against the wrong match/display block?

Unclear how that'd work in this case so if you could include a worked example it would be useful. This also appears to be against the wrong match/display block?
freyjadomville marked this conversation as resolved
dragonmux force-pushed feature/full-bmp-rsp-support from d104cefa6b
All checks were successful
ci/woodpecker/pr/build-windows/2 Pipeline was successful
ci/woodpecker/pr/build-linux Pipeline was successful
ci/woodpecker/pr/build-windows/1 Pipeline was successful
to 7993bc098a
All checks were successful
ci/woodpecker/pr/build-windows/2 Pipeline was successful
ci/woodpecker/pr/build-linux Pipeline was successful
ci/woodpecker/pr/build-windows/1 Pipeline was successful
ci/woodpecker/push/build-windows/2 Pipeline was successful
ci/woodpecker/push/build-linux Pipeline was successful
ci/woodpecker/push/build-windows/1 Pipeline was successful
2026年05月06日 02:31:45 +02:00 Compare
Sign in to join this conversation.
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
blackmagic-debug/bmputil!82
Reference in a new issue
blackmagic-debug/bmputil
No description provided.
Delete branch "feature/full-bmp-rsp-support"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?