Skip to content
This repository was archived by the owner on Mar 7, 2026. It is now read-only.

Improved BMDA probe scanning#1502

Merged
dragonmux merged 25 commits into
blackmagic-debug:mainfrom
sidprice:feature/improved-bmda-probe-scan
Jul 20, 2023
Merged

Improved BMDA probe scanning#1502
dragonmux merged 25 commits into
blackmagic-debug:mainfrom
sidprice:feature/improved-bmda-probe-scan

Conversation

@sidprice
Copy link
Copy Markdown
Contributor

@sidprice sidprice commented May 31, 2023

This PR is a complete rewrite of the BMDA probe scanner and also includes an implementation for FTDI-based probes on Windows. The latter does not require and special driver installation.

  • I've read the Code of Conduct
  • I've read the guidelines for contributing to this repository
  • It builds for hardware native (make PROBE_HOST=native)
  • It builds as BMDA (make PROBE_HOST=hosted)
  • I've tested it to the best of my ability
  • My commit messages provide a useful short description of what the commits do

Copy link
Copy Markdown
Member

@dragonmux dragonmux left a comment

Choose a reason for hiding this comment

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

Thank you for the contribution! What a major piece of work, and an awesome thing to get sorted out.

As talked about in DMs, this is only a first pass of review, and we expect to have to do several more as things get ironed out and rationalised.

Comment thread src/platforms/hosted/windows/ftdi.c Outdated
Comment thread src/platforms/hosted/windows/ftdi.c Outdated
Comment thread src/platforms/hosted/windows/ftdi.c Outdated
Comment thread src/platforms/hosted/windows/ftdi.c Outdated
Comment thread src/platforms/hosted/windows/ftdi.c Outdated
Comment thread src/platforms/hosted/windows/ftdi.c Outdated
Comment thread src/platforms/hosted/windows/ftdi.c Outdated
Comment thread src/platforms/hosted/windows/ftdi.c Outdated
Comment thread src/platforms/hosted/windows/ftdi.c Outdated
Comment thread src/platforms/hosted/windows/ftdi.c Outdated
@sidprice sidprice force-pushed the feature/improved-bmda-probe-scan branch from 090e490 to ca73e0f Compare June 1, 2023 16:29
@dragonmux dragonmux added Enhancement General project improvement BMD App Black Magic Debug App (aka. PC hosted) (not firmware) labels Jun 1, 2023
@dragonmux dragonmux added this to the v1.10 milestone Jun 1, 2023
@sidprice sidprice force-pushed the feature/improved-bmda-probe-scan branch 2 times, most recently from de0f7f1 to c0c6591 Compare June 1, 2023 21:19
Copy link
Copy Markdown
Member

@dragonmux dragonmux left a comment

Choose a reason for hiding this comment

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

As promised, here's our second review pass - trying to complete review of all FTDI code so we can get into the meat and potatoes of the probe scanning rework next round

Comment thread src/platforms/hosted/ftdi_bmp.c Outdated
Comment thread src/platforms/hosted/ftdi_bmp.c Outdated
Comment thread src/platforms/hosted/ftdi_bmp.c Outdated
Comment thread src/platforms/hosted/ftdi_bmp.c Outdated
Comment thread src/platforms/hosted/ftdi_bmp.c Outdated
Comment thread src/platforms/hosted/ftdi_bmp.c Outdated
Comment thread src/platforms/hosted/ftdi_bmp.c Outdated
Comment thread src/platforms/hosted/platform.h Outdated
@sidprice sidprice force-pushed the feature/improved-bmda-probe-scan branch from c0c6591 to 310c138 Compare June 6, 2023 13:21
@dragonmux dragonmux force-pushed the feature/improved-bmda-probe-scan branch 8 times, most recently from dfec127 to 4a7e660 Compare June 8, 2023 12:23
@sidprice sidprice force-pushed the feature/improved-bmda-probe-scan branch 2 times, most recently from 52a6d35 to fd16f59 Compare June 9, 2023 17:33
@sidprice sidprice force-pushed the feature/improved-bmda-probe-scan branch 2 times, most recently from d2b901b to 616d27f Compare June 30, 2023 23:06
Copy link
Copy Markdown
Member

@dragonmux dragonmux left a comment

Choose a reason for hiding this comment

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

We have done a coarse review of bmp_libusb.c now (sorry it took so long to get to) and we'll do a further review after you've had a chance to work through the points raised in this one.

Comment thread src/platforms/hosted/bmp_libusb.c Outdated
Comment thread src/platforms/hosted/bmp_libusb.c Outdated
Comment thread src/platforms/hosted/bmp_libusb.c Outdated
Comment thread src/platforms/hosted/bmp_libusb.c Outdated
Comment thread src/platforms/hosted/bmp_libusb.c Outdated
Comment thread src/platforms/hosted/bmp_libusb.c Outdated
Comment thread src/platforms/hosted/bmp_libusb.c Outdated
Comment thread src/platforms/hosted/bmp_libusb.c Outdated
Comment thread src/platforms/hosted/bmp_libusb.c Outdated
Comment thread src/platforms/hosted/bmp_libusb.c Outdated
Comment thread src/platforms/hosted/bmp_libusb.c Outdated
Comment thread src/platforms/hosted/bmp_libusb.c Outdated
Comment thread src/platforms/hosted/bmp_libusb.c Outdated
@sidprice sidprice force-pushed the feature/improved-bmda-probe-scan branch 5 times, most recently from d4e0c54 to 34f1822 Compare July 13, 2023 21:57
@dragonmux dragonmux force-pushed the feature/improved-bmda-probe-scan branch from 65f2562 to ad5e834 Compare July 19, 2023 06:36
@dragonmux dragonmux force-pushed the feature/improved-bmda-probe-scan branch 2 times, most recently from 3efdc7a to 1e6193b Compare July 19, 2023 19:29
@dragonmux dragonmux force-pushed the feature/improved-bmda-probe-scan branch from 1e6193b to d8fe188 Compare July 19, 2023 19:32
@sidprice sidprice force-pushed the feature/improved-bmda-probe-scan branch from d8fe188 to b03aac9 Compare July 19, 2023 22:42
@dragonmux dragonmux force-pushed the feature/improved-bmda-probe-scan branch from b03aac9 to daecdef Compare July 19, 2023 22:46
@dragonmux dragonmux requested a review from esden July 19, 2023 22:51
Copy link
Copy Markdown
Member

@dragonmux dragonmux left a comment

Choose a reason for hiding this comment

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

This all looks good to us now, so handing this over to Esden for final review seeing we've touched the files ourself too much.

@sidprice
Copy link
Copy Markdown
Contributor Author

@dragonmux Agreed, this was a great team effort!

Copy link
Copy Markdown
Member

@esden esden left a comment

Choose a reason for hiding this comment

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

This looks great. Thanks for all the hard work. I don't have comments at the moment without spending way too much time stifling progress here. :D I did make a scan over the changes and it is a great improvement over all. Thank you @sidprice and @dragonmux for all the hard work.

@dragonmux dragonmux dismissed their stale review July 20, 2023 06:40

Taken care of, hand-over to Esden

@dragonmux dragonmux merged commit 00b3e07 into blackmagic-debug:main Jul 20, 2023
@sidprice sidprice deleted the feature/improved-bmda-probe-scan branch July 22, 2023 21:03
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

BMD App Black Magic Debug App (aka. PC hosted) (not firmware) Enhancement General project improvement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants