Skip to content

Conversation

@PathogenDavid
Copy link
Collaborator

@PathogenDavid PathogenDavid commented Jul 14, 2025

This PR shows the minimal(ish) changes needed to get the Pico Core working with Harp Regulator.

Ideally you probably want to tidy these things up.

In particular, the new registers need to be ratified (harp-tech/harp-regulator#2) and implemented in the core rather than main.cpp.

I think you also probably don't want the redundancy of the WhoAmI and firmware version all over the place. Ideally both of these should probably just be defined once in the CMakeLists.txt and propagated to where they're needed as appropriate.

I think I mentioned it elsewhere, but it is worth noting that the USB product name (iProduct) is effectively the only place that we can put info in the USB descriptor and have it be able to be read on Windows. The manufacturer and serial number strings simply aren't reliably exposed anywhere as far as I could find. (Manufacturer in particular I think is due to Windows preferring to show the vendor who made the driver. No idea why they don't expose the serial number.)

Also it's slightly more important that the Pico Core uses the flash ID on RP2040 and the chip ID on RP2350 as the serial number now. Harp Regulator relies on the serial number matching to tell that a BOOTSEL device is the same as the device it instructed to reboot. (Getting the flash ID via Picoboot was the most straightforward way to accomplish this.)

@Poofjunior
Copy link
Collaborator

Nicely done!

I don't think there's any downside to just using the max usb descriptor character limit AFAIK. We'll need to update the naming conventions to adopt HARP_DEVICE_DESCRIPTION, but I think that's ok given the Windows compatibility issue.

@Poofjunior Poofjunior marked this pull request as ready for review July 17, 2025 19:28
@Poofjunior Poofjunior merged commit dded147 into harp-tech:main Jul 17, 2025
3 checks passed
@bruno-f-cruz
Copy link
Member

@Poofjunior Was this meant to be merged? It seems like it would greatly benefit from unblocking via harp-tech/harp-regulator#2 as the original issue stated.

For context, this discussion was started here but never resulted in a proper proposal. We can start with the current spec as an initial draft, but ideally end with something that could be generically used for multiple architecturess

@Poofjunior
Copy link
Collaborator

@bruno-f-cruz you're right. I was treating this as a standalone demo with functionality added as app registers, but I'll revert so we can discuss how to fold the functionality currently proposed as registers above. Thanks for the catch!

@PathogenDavid
Copy link
Collaborator Author

Haha, yeah I was surprised to see this got merged. I should've clarified that I this draft PR was submitted more as a way to share my minimal demo rather than something meant to be merged as-is, sorry about the confusion! 😅

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.

3 participants