Support for serial port "touch" functionality using libserialport #1507
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This makes it possible to upload to boards like the Arduino Leonardo and Arduino Nano Every wwithout using an external tool to enter bootloader mode
|
It seems like the CI is complaining for some reason. It builds just file using both make and cmake on my computer... |
|
I think github action failed because libserial port is not installed. This was left not done in the previous PR. For now, I think we can update github action script to install libserialport for Linux, macOS and MinGW, and we have to disable libserialport for MSVC for now. |
|
I can build under MSYS2 mingw64 since I have libserialport installed. But there is a warning. |
|
Build log under Visual Studio 2022 (no libserialport installed) -- building step will fail. Configuration step: Building step: |
|
Probably one simple fix is to guard |
|
Thanks for the heads up @mcuee! And since @kristofmulier has requested this functionality earlier, maybe he wants to try it as well? |
Great. This seems to work fine under Windows (COM port changes from COM8 to COM9 during the test. Arduino 1.89 with avrdude 6.3 output for reference as well. |
Excellent test! Confirming that this (worst-case scenario) works fine on Windows makes me think that this might be a reliable solution to the problem.
EDIT: Found it: SerialUploader.java. I'll go through it to see if there are any quirks I need to adopt to make sure the functionality in this PR works reliably on all platforms. |
|
@mcuee @stefanrueger I've been thinking about removing the user-specifiable baud rate and only supporting At the moment, neither Arduino IDE nor PlatformIO supports other baud rates than 1200 for doing a "port touch". |
|
Great! It's awesome to see that you guys are integrating this functionality in the official AVRDUDE. Any idea when it will be in the new release? |
AFAIK, all boards/bootloaders that are using this approach to enter programming mode are using 1200 baud 8N1 anyways
|
I've removed the user-specifiable touch baud rate in the last commit, as pretty much every application would be using
Avrdude 7.2 is quite new, so I'd say at least 4 or 5 months before a new version gets released. |
I agree. The latest commit is still good under Windows. |
As mentioned by Hans, we will not have avrdude 7.3 release so soon. You can always give this PR and git main a try. |
|
Somehow it does not work under Linux (Ubuntu 20.04). Arduino 1.89 with avrdude 6.3 is okay. |
|
Maybe a bit of delay is necessary under Linux. |
That's weird. If you look at the source code, you can see that there's already a 1-second sleep ( int touch_serialport(char **portp, int baudrate) {
int n1, n2;
SERPORT *sp1, *sp2, **diff;
sp1 = get_libserialport_data(&n1);
if(!sp1 || n1 <= 0)
return -1;
pmsg_info("touching serial port %s at %d baud\n", *portp, baudrate);
struct sp_port *prt;
struct sp_port_config *cfg;
sp_get_port_by_name(*portp, &prt);
sp_open(prt, SP_MODE_READ);
sp_set_baudrate(prt, baudrate);
sp_set_bits(prt, 8);
sp_set_parity(prt, SP_PARITY_NONE);
sp_set_stopbits(prt, 1);
sp_set_flowcontrol(prt, SP_FLOWCONTROL_NONE);
sp_new_config(&cfg);
sp_get_config(prt, cfg);
sp_set_config_dtr(cfg, SP_DTR_OFF);
sp_close(prt);
sp_free_port(prt);
usleep(1000000);
for(int i = 0; i < 5; i++) {
sp2 = get_libserialport_data(&n2);
diff = sa_spa_not_spb(sp2, n2, sp1, n1);
if(*diff && (*diff)->port) {
pmsg_notice("new port %s discovered\n", (*diff)->port);
if(portp) {
if(*portp)
free(*portp);
*portp = cfg_strdup(__func__, (*diff)->port);
}
free(diff);
break;
}
free(diff);
usleep(500000);
}
return 0;
}EDIT: if we should do it exactly like Arduino IDE does it, then the first sleep should be reduced from 1s to 400ms, and the second sleep (inside the for loop) from 500 to 250ms. Can you try this @mcuee? |
|
Still it does not work. Strange. |
|
I played a bit with other delay combinations and they do not work either. I will try another Linux machine tomorrow to see if that makes a difference. Or maybe @stefanrueger can take a look at the codes to see if there is something to take note. |
|
I had a quick look; well-programmed (but will do a proper review later). Re timings. I realise this is modelled after the Arduino IDE, but we should figure out cross-platform timings ourselves.
The loop assumes that the port changes by touching with 1200 baud. But does the port change in all OSes and all circs? If it doesn't the we attract 1000 ms + 5 · 500 ms sleep, 3.5 s. Maybe try running the loop only once (but not more twice) and tweak both sleep times so that they work across all platforms. Or have platform dependent sleeps. How long does it take for the port to spring up? I would have naively thought if a new port is not there by 500 ms then it never will. |
|
This works fine under Ubuntu Linux 20.04 x86_64 (my Windows11/Ubuntu 20.04 dual boot Acer Swift 3 year 2021 laptop). The good thing is that consecutive runs of avrdude (keep running the same command) are also fine. |
|
It is also find on Windows 11 on the same Acer laptop. |
|
Somehow |
|
@stefanrueger But probably need to exclude Apple Silicon which is also ARM64. Edit to add: indeed it affects my Mac Mini M1 to make it too long (1250ms). The following patch is better which reduce the delay to 750ms for the Mac Mini M1. |
Somehow this PR does not work with official Arduino Nano Every board, under Windows. The older method works. The following two experiments do not work either.
|
But it works perfectly under Linux, including my Acer laptop and Raspberry Pi 400. Edit: it also works with my Mac Mini M1. |
Thanks for testing @mcuee! I have lost track: which older method do you refer to? So we can identify the difference.
OK. I have now changed to Handing back to @MCUdude and @mcuee for the final round of checks and adaptations. |
The old method is mentioned in the above post. It is very strange that this PR does not work with Arduino Nano Every on Windows, yet no issues under Linux and macOS. The old method seems pretty reliable under Windows. For this PR, maybe it is again some timing issues. But I have not been able to figure out a work-around. Take note there is no PORT change and I do not hear Windows beep sound (no USB renumeration), either with the old working method, or this PR. |
|
My really, really slow Raspberry Pi 1 running Raspberry Pi OS works with both the Nano Every and the Pro Micro. When connecting to the Nano Every the port stays the same so it connects to the same port, which does work. |
|
Please help to check if you can reproduce the issue of Arduino Nano Every board under Windows. Thanks. That is the only remaining issues I have with this PR. |
|
It seems to me that somehow the current touch method disturb the jtag2updi programmer. Working method: This PR: here I am trying the reduced time of 100ms; same result as decault 400ms. I have also tried to have longer delay. |
Sorry, for the delay. I'll see if I can give it a try this evening.
Strange... And you're using an Arduino Nano Every, not a DIY jtag2updi? |
Correct. I am using the official Arduino Nano Every board, not a DIY jtag2updi. And the issue only happens under Windows. It is okay under Linux and macOS. I will try with a DIY jtag2updi as well later. |
Thanks for the tip. No issues with a jtag2updi using a CH340 Arduino Uno clone. |
|
I am good with the PR now. I will attribute the issue with official Arduino Nano Every board under Windows to the on-board jtag2updi implementation by Arduino. If there is an easy fix, that would be good. If not, I can live with that problem under Windows. Reference: Arduino jtag2updi implementation for Arduino Nano Every There are multiple issues from Arduino github repo: |
|
I have found the fix for the Arduino Nano Every problem under Windows. I believe the fix is safe to be added. Please check as well. Thanks. |
|
@mcuee Good find! I suggest a very short delay when plucking RTS/DTR (see discussion in PR #1505) I pushed a relevant commit. Please re-test this version for all combos (as it is a fundamental change in the core serial touch routine). |
Yes this is good for all three setup I have. Now I am fine with this PR.
|
|
Great find @mcuee! The Nano Every still works on Mac OS. BTW, avrdude doesn't suggest a serial port if the user specified is not present when using |
Good catch. I pushed a commit; it should now. |
|
Excellent @stefanrueger! I think this PR safely can be merged in the next merge fest. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This draft PR makes it possible to upload to boards like the Arduino Leonardo and Arduino Nano Every without using an external tool to enter "upload mode".
This PR is a result of some test code I hacked together that turned out to work quite well. I'm open to suggestions on how this can be further improved.
Tested on an Arduino Pro Micro (running the Arduino Leonardo bootloader) and the Arduino Nano Every.
The Pro Micro has a different USB PID when in bootloader mode, while the Nano Every stays constant.
Resolves #1443