-
Notifications
You must be signed in to change notification settings - Fork 125
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix probing of the generic -c stk500 programmer #1468
Conversation
I'm getting a segmentation fault. And I promise I've cleaned everything and done a fresh build.
|
I guess stk500v1 has added a setup call in the last decade; have added the setup() call before probing stk500v1. Try now |
Now Ideally, it should try with stk500v2 first, and if this times out, switch to stk500v1. As you can see, it took almost a minute to connect
|
Indeed it is pretty fast for STK500v1.
And indeed very slow for STK500v2. In fact, it causes issues with my CH340 STK500v2 clone, which is a regression.
The clone has no issues with git main with either
|
Since STK500 V1 FW is rare, I tend to agree with @MCUdude here. And this is more inline with git main now. Since this change causes regression for my CH340 STK500v2 clone, I think it is better to follow the suggestion from @MCUdude . |
OK, makes sense to probe v2 first then v1. I added the matching teardown call given the respective programmer's setup() call. Warrants checking with
|
The problem now is that there isn't a limit to how many times Avrdude tried to connect to an stk500v2 target, and it takes somewhere between 5 and 10 seconds between each retry attempt. It also spits out a false "successfully opened stk500v2 device; please use -c stk500v2" message. (STK500 board running stk500v1 firmware):
|
Aha, so the intended probing algorithm never would have worked, because stk500v2_open() returns 0 despite comms failure. have repaired stk500v2_open() in that respect and stk600v2_open() too (while I was at it). So, someone should try an STK600 programmer once. |
It now works with the stk500v1, but it takes quite a while before it switches to stk500v1. But that's fine I think. At least better than before!
With a board running stk500v2 firmware, it's just quick as one would expect:
The STK600 also works:
|
…ailure ... as it might have been that probing with stk500v2 protocol threw a stk500v1 firmware off its rails
... which means that some genuine stk500v1-protocol programmer might at this point already have given up comms and no longer synch. Hence, I think we should change the error message "cannot open either stk500v1 or stk500v2 programmer" to "probing stk500v2 failed, as did stk500v1; perhaps try -c stk500v1" |
It seems to me avrdude 5.2 version works very well. Maybe we can follow its behaviour if possible. Basically it tries to detect STK500V1 first (one retry??) and then try the V2. The high number of retrys probably only for the bootloader mode and maybe Good thing is that all the avrdude version from the following sites still work under my Ubuntu 20.04 machine (including 4.4 version which was released back in 19-July-2004). Kudos to @dl8dtl and Brian S Dean. It has short delay for STK500 V2 devices (maybe only one retry, I have not looked at the codes).
It has almost no delay with STK500 V1 devices.
BTW, it seems to me |
I am in general okay with this PR (to detect V2 first and then V1) but I would like to reduce the retry count to 1 or 2 if the user uses Note;
|
The stk500v2 implementation has a hard-coded number of timeouts. The stk500v1 has a user specifiable timeout count |
I think we can change the behavior for real programmers. We can keep the higher number of retry for bootloaders. But maybe we can ask @dl8dtl to see why we have a hard coded retry count. And it is good to have a user specifiable retry count.
|
This PR is of course better than git main. But I will compare with old avrdude 5.2 version. Test with Arduino as ISP.
|
Comparison with avrdude 5.2 under Linux, for "Arduino as ISP". I think this PR is okay and it behaves better under Linux (it can recover) than under Windows (it can not recover) when using
|
I am still thinking that we should probably reduce the number of retry count for stk500v2 probe, for real programmers (not for bootloaders). It would be good to have a retry count just like |
I have always been suspicious about retrying without changes and rarely seen examples where that works. Given that programmers do different things at startup there is no simple rule, and limiting the retry count is rarely a success. |
Same here. I tend to think 3 or more retries do not help in general.
Okay then. |
I am okay with this PR even though I still prefer just to change the documentation to say now |
@stefanrueger |
Fixes #1467