Skip to content

Conversation

@jonathanherbstgrapple
Copy link
Contributor

@jonathanherbstgrapple jonathanherbstgrapple commented Oct 15, 2024

Process SWD sequence commands to pass them down to the device.

I changed the version to 0.2.0 instead of 0.1.1 because I modified the Swd trait, which would break dependent crates.

Changes:

  • Added code to process an swd sequence command and translate to swd trait functions
  • Modified the Swd trait to support swd sequence commands
  • Added mocking to be able to test the dap protocol without hardware

Testing:

  • Unit tests to test some cases of the swd sequence commands.

Related Issues:

@korken89
Copy link
Collaborator

Hi Jonathan,

Thank you for implementing this addition, is was really missing!
I'll review during tomorrow. Feel free to hit me up on Matrix if you want to discuss something sync.

Copy link
Collaborator

@korken89 korken89 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Over all this looks good! Is there any implementation using this for reference?

I'm in the process of getting the Rusty Probe upgraded to latest HAL, after that I can try implementing this to make sure it works.

@jonathanherbstgrapple
Copy link
Contributor Author

It's not tested other than the unit tests.

@jonathanherbstgrapple
Copy link
Contributor Author

Over all this looks good! Is there any implementation using this for reference?

I'm in the process of getting the Rusty Probe upgraded to latest HAL, after that I can try implementing this to make sure it works.

Let me know if there's anything I can do to help upgrading to the latest hal.

@korken89
Copy link
Collaborator

If you have a probe, feel free to give it a try: https://github.com/probe-rs/rusty-probe-firmware/tree/update-rp-hal
I think I have finished the update, but I don't have hardware at home to test on.

I'll need to get my testing setup up and running again. :)

@korken89
Copy link
Collaborator

Was this tested based on the new HAL branch?

@korken89
Copy link
Collaborator

Can you rebase this on master? Then I'll have it merged and released 👍

@korken89
Copy link
Collaborator

I pushed the fixes to https://github.com/probe-rs/dap-rs/tree/Grapple-Systems-add-swd-sequence you can steal my commit :)

@jonathanherbstgrapple
Copy link
Contributor Author

Can you rebase this on master? Then I'll have it merged and released 👍

I pushed the fixes to https://github.com/probe-rs/dap-rs/tree/Grapple-Systems-add-swd-sequence you can steal my commit :)

Done, your changes look fine to me.

@jonathanherbstgrapple
Copy link
Contributor Author

Don't merge this, I think it's wrong. I missed the sequence count and the fact you can have multiple sequences.

@korken89
Copy link
Collaborator

Alright! Ping me when you want feedback

@jonathanherbstgrapple
Copy link
Contributor Author

Fixed. I guess the next step is to implement swd sequence reading and writing through the rusty probe firmware and test with something that already does swd sequences correctly.

@jonathanherbstgrapple
Copy link
Contributor Author

jonathanherbstgrapple commented Oct 30, 2024

I went ahead and did a bitbanged implementation of this in the rusty probe firmware, you can check it out here:
https://github.com/jonathanherbstgrapple/rusty-probe-firmware/tree/swd-sequence-working

That branch also has a hacked fix for probe-rs/rusty-probe-firmware#26

There's one piece of this I'm still not 100% sure on. If you put two read sequences in the same command I don't know if they get packed together within the same byte or if each one starts on a new byte. I currently have them starting on a new byte, which seems sane to me, but I'll spend some time reading the cmsis dap library to figure out what the right thing to do is.
CMSIS DAP does input responses the way I implemented it.

@korken89
Copy link
Collaborator

korken89 commented Nov 2, 2024

Thank you for fixing this!

@korken89 korken89 merged commit 7d36867 into probe-rs:master Nov 2, 2024
@korken89
Copy link
Collaborator

korken89 commented Nov 2, 2024

I'll prepare a release of 0.2 so we can use it in rusty-probe-firmware

@korken89
Copy link
Collaborator

korken89 commented Nov 2, 2024

Released! I'll look into your commits and adding them.

@korken89
Copy link
Collaborator

korken89 commented Nov 2, 2024

I gave your commits a go, seems to be working fine.

Is there anything you are having issues with?
I only added the changes you made to src/dap.rs.

@korken89
Copy link
Collaborator

korken89 commented Nov 2, 2024

I cherry-picked your changes, and fixed it up a little: probe-rs/rusty-probe-firmware#29
I think it's ready for merge, feel free to comment on the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants