Skip to content

Conversation

rhn
Copy link

@rhn rhn commented May 5, 2025

This is a rebase of the rebase #36 of the original #25.

I tested it with the serial example from Disasm/usb-otg-workspace#7 (also had to rebase that, available upon request).

I realize testing is the problem with merging this. Sadly, I don't have that many devices, so I couldn't test across the entire range.

I think this feature is really useful, and seeing that this is the third time it's been proposed, I'm not the only one. I am willing to put in extra effort to have it merged in some form, rather than wait another 2 years waiting for testing that the community is unable to provide.

So if this cannot be merged the way it is now, I propose a solution:

I create a feature "gd32v103", which contains these changes. It is not enabled by default, so existing device support is unaffected. People who want to use this crate on GD32 microcontrollers must enable the feature. People who want to test the new code on STM32 controllers can enable this feature. Once all devices have been tested, the old code is removed and the feature is removed, too.

Would this solution make it mergeable?

@dcz-self
Copy link

@Disasm are you also the maintainer of this one? I already tested this and it's actually working.

@Disasm
Copy link
Member

Disasm commented Aug 27, 2025

@dcz-self yes, unfortunately.

I already tested this and it's actually working.

Tested on what hardware?

@dcz-self
Copy link

I tested this on GD32VF103.

@Disasm
Copy link
Member

Disasm commented Aug 28, 2025

I have a vacation in September and I'm going to invest some time (starting from 2025-09-08) into testing other targets as there is a risk of regressions there. Does this sound good?

@dcz-self
Copy link

Anything is great, honestly, but I wouldn't want to put any pressure on you - I'm sure there were good reasons why you couldn't contribute.
I'm ready to take over work from you instead. I won't be able to test all the targets, but I could do some steps, like:

  • make a major version bump and wait for bug reports, or
  • keep users of this crate on this crate by making it deprecated and point people to a new one which moves faster but has less testing, until it picks up more people.

Honestly, I don't want to take your vacation away.

@Disasm
Copy link
Member

Disasm commented Aug 28, 2025

Maintenance is done in my free time, so it doesn't matter much whether this free time is consumed during vacations or during weekend, but it's just more convenient for me to perform testing during vacation since testing takes a lot of time and I can concentrate on it better this way.

I was thinking about releasing a semver-incompatible version of this crate with GD32 support, but then testing burden will move to maintainers of the hal crates, and this is something I'd like to avoid.
I don't see a point of making this crate deprecated since it still works, even though it's not well-maintained and there is no better-maintained replacement in sight. In the past there was an attempt to create a fork of another USB crate (stm32-usbd -> stm32-usbd2) to achieve the goal of releasing untested changes, but as you can see from crates.io no other crate depends on this fork for some reason. You can also make your own fork and release it if you wish, but I believe that in general there is more value in providing tested crates than the newest changes. In this regard what we can do is to put more maintenance effort into it by providing reviews and testing. Ideally one can create a test jig so that testing (at least partially) can be performed automatically. This approach was taken, for example, for probe-rs, but not for USB crates yet.

In any case, maintenance is hard, and I'm happy to receive any help with it and discuss any ideas on how to improve the situation. For example, the gd32v103 feature proposed in this PR looks like a good intermediate solution, especially since most of the changes are not related to other targets, so only a few of them need to be guarded. This way we can skip testing other targets and release a new version. However, testing will still be needed later to remove the feature.

@dcz-self
Copy link

You wrote "unfortunately", so I got an impression you would prefer to not maintain this at all.

That's why I proposed "deprecated". Maybe that's not the greatest word: the crate is stable and useful, but not updated due to the lack of resources (time, hardware), so it's useless for those who have needs unfulfilled by it today. Few contributors have all supporting hardware, and few maintainers working for free are going to maintain a lab full of testing devices as well. I think it's commendable of you to put in this kind of effort, but I suspect you already burned out because of that ("testing takes a lot of time").

That's why I think there's space for a less-tested version of the same. I almost just gave up on my project before starting when I discovered the available crates need patching.

no other crate depends on this fork for some reason

Forks aren't easily discoverable. In the "unstable fork" scenario, I wouldn't even begin without putting a prominent notice on te old crate along the lines of "this crate is in low-maintenance mode. For less stable improvements go to this new crate".

I think releasing a feature is a good compromise.

Meanwhile, I need to get to work and I'm unlikely to contribute code/reviews until the 15th.

@Disasm
Copy link
Member

Disasm commented Aug 28, 2025

You wrote "unfortunately", so I got an impression you would prefer to not maintain this at all.

Nope, this is because I don't have enough time and energy at the moment to maintain it and I realize this creates problems for many. I would actually prefer to continue maintaining it since I have a lot of related hardware and software needed for testing, and quite often people contributing to this project have only the board they need support for.

but I suspect you already burned out because of that ("testing takes a lot of time").

It actually takes a lot of time :D For a reference, these are the tests I executed last time for the 0.4.0 release:
image

Forks aren't easily discoverable. In the "unstable fork" scenario, I wouldn't even begin without putting a prominent notice on te old crate along the lines of "this crate is in low-maintenance mode. For less stable improvements go to this new crate".

True, but this specific crate was advertised within Embedded WG. I fully support your idea with adding a notice referencing an alternative crate.

@dcz-self
Copy link

dcz-self commented Aug 28, 2025

That's an impressive table. Long-term I imagine it only makes sense to have it as an automated lab. The cost of testing is going to keep increasing as a crate gets wider support, until at some point it's no longer possible to merge anything based only on volunteer work, and the crate dies. Which is what I suspect kind of almost happened here.
Actually, this also happened to me in a different context in paid work too.

Anyway, I hope the current way forward works, and if not, I'm ready to help start an unstable crate.

@Disasm
Copy link
Member

Disasm commented Aug 28, 2025

Sure it's beneficial to have an automated setup in a long run, but someone has to create and maintain it. I made a prototype 5 years ago (https://github.com/Disasm/usb-tester) for testing the other subset of STM32 MCUs, but it doesn't support a lot of features needed to test the USB-OTG driver, including JTAG needed for GD32V. And this setup only partially supports compliance testing, since all of it currently requires a Windows laptop with specialized software. A lot of effort would be required to reverse-engineer this software it order to run it on Linux or an MCU, which is required to be able to run the compliance tests in CI.

Let's continue with the following plan. I'll try to test the GD32V change against other targets. If no regressions found, I'll merge the change and create a release. If regressions found, I'll switch to the gd32v103 feature idea proposed here and create a release with it. If release doesn't happen for some reason before you have time to work on this crate, feel free to fork and make a release with whatever changes you need.

@Disasm
Copy link
Member

Disasm commented Sep 8, 2025

Closing since the base PR is now merged. Thank you for the input!

@Disasm Disasm closed this Sep 8, 2025
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