Skip to content
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

Support for serial port "touch" functionality using libserialport #1507

Merged
merged 19 commits into from
Oct 15, 2023
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions src/avrdude.1
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
.Nm
.Fl p Ar partno
.Op Fl b Ar baudrate
.Op Fl r
.Op Fl B Ar bitclock
.Op Fl c Ar programmer-id
.Op Fl C Ar config-file
Expand Down Expand Up @@ -365,6 +366,11 @@ for more information run avrdude -p x/h.
.It Fl b Ar baudrate
Override the RS-232 connection baud rate specified in the respective
programmer's entry of the configuration file.
.It Fl r
Opens the serial port at 1200 baud and immedeatly closes it, prior to
establishing communication with the programmer. This is commonly knows as A
Copy link
Collaborator

Choose a reason for hiding this comment

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

known as a

"1200bps touch", and is used to trigger programming mode for certain boards
like Arduino Leonardo, Arduino Micro/Pro Micro and the Arduino Nano Every.
.It Fl B Ar bitclock
Specify the bit clock period for the JTAG, PDI, TPI, UPDI, or ISP
interface. The value is a floating-point number in microseconds.
Expand Down
6 changes: 6 additions & 0 deletions src/doc/avrdude.texi
Original file line number Diff line number Diff line change
Expand Up @@ -469,6 +469,12 @@ ATmega328P MCU properties; for more information run @code{avrdude -p x/h}.
Override the RS-232 connection baud rate specified in the respective
programmer's entry of the configuration file.

@item -r @var{baudrate}
Copy link
Collaborator

@stefanrueger stefanrueger Oct 3, 2023

Choose a reason for hiding this comment

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

remove @var{baudrate}

Opens the serial port at 1200 baud and immedeatly closes it, prior to
establishing communication with the programmer. This is commonly knows as A
Copy link
Collaborator

Choose a reason for hiding this comment

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

known as a

"1200bps touch", and is used to trigger programming mode for certain boards
like Arduino Leonardo, Arduino Micro/Pro Micro and the Arduino Nano Every.

@item -B @var{bitclock}
Specify the bit clock period for the JTAG, PDI, TPI, UPDI, or ISP
interface. The value is a floating-point number in microseconds.
Expand Down
1 change: 1 addition & 0 deletions src/libavrdude.h
Original file line number Diff line number Diff line change
Expand Up @@ -1252,6 +1252,7 @@ extern "C" {
int setport_from_serialadapter(char **portp, const SERIALADAPTER *ser, const char *sernum);
int setport_from_vid_pid(char **portp, int vid, int pid, const char *sernum);
int list_available_serialports(LISTID programmers);
int touch_serialport(char **portp, int baudrate);

int str_starts(const char *str, const char *starts);
int str_eq(const char *str1, const char *str2);
Expand Down
12 changes: 11 additions & 1 deletion src/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,8 @@ static void usage(void)
" -p <wildcard>/<flags> Run developer options for matched AVR devices,\n"
" e.g., -p ATmega328P/s or /S for part definition\n"
" -b <baudrate> Override RS-232 baud rate\n"
" -r Open and close (\"touch\") the serial port at\n"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Perhaps say why this is needed? The manual can (and does) go into the gory details.

"Touch" the serial port before programming; needed for XXX, YYY etc bootloader uploads

" 1200 baud before establishing connection\n"
" -B <bitclock> Specify bit clock period (us)\n"
" -C <config-file> Specify location of configuration file\n"
" -c <programmer> Specify programmer; -c ? and -c ?type list all\n"
Expand Down Expand Up @@ -526,6 +528,7 @@ int main(int argc, char * argv [])
char * e; /* for strtod() error checking */
const char *errstr; /* for str_int() error checking */
int baudrate; /* override default programmer baud rate */
bool touch_1200bps = false; /* "touch" serial port prior to programming */
double bitclock; /* Specify programmer bit clock (JTAG ICE) */
int ispdelay; /* Specify the delay for ISP clock */
int init_ok; /* Device initialization worked well */
Expand Down Expand Up @@ -635,7 +638,7 @@ int main(int argc, char * argv [])
/*
* process command line arguments
*/
while ((ch = getopt(argc,argv,"?Ab:B:c:C:DeE:Fi:l:np:OP:qstT:U:uvVx:yY:")) != -1) {
while ((ch = getopt(argc,argv,"?Ab:B:c:C:DeE:Fi:l:np:OP:qrstT:U:uvVx:yY:")) != -1) {

switch (ch) {
case 'b': /* override default programmer baud rate */
Expand Down Expand Up @@ -766,6 +769,10 @@ int main(int argc, char * argv [])
quell_progress++ ;
break;

case 'r' :
touch_1200bps = true;
break;

case 't': /* enter terminal mode */
ladd(updates, cmd_update("interactive terminal"));
break;
Expand Down Expand Up @@ -1231,6 +1238,9 @@ int main(int argc, char * argv [])
free(port_tok[i]);
}

if(touch_1200bps && pgm->conntype == CONNTYPE_SERIAL)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe move into the preceding if(pgm->conntype == CONNTYPE_SERIAL) block? Simplifies code

if(touch_1200bps)
  touch_serialport(&port, 1200);

(and saves an empty line)

Copy link
Collaborator

@stefanrueger stefanrueger Oct 3, 2023

Choose a reason for hiding this comment

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

And, of course, we should resolve whether a failed touch is

  • Fatal: use pmsg_error() in touch_serialport() and exit:
    if(touch_1200bps && touch_serialport(&port, 1200) != 0)
       exit(1);
    
  • Advisory only: ignore failure but then use pmsg_warning() in touch_serialport()

I lean towards the first as -r was requested and could not be delivered.

touch_serialport(&port, 1200);

/*
* open the programmer
*/
Expand Down
65 changes: 57 additions & 8 deletions src/serialadapter.c
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include <string.h>
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>

#include "avrdude.h"
#include "libavrdude.h"
Expand Down Expand Up @@ -130,6 +131,13 @@ static SERPORT *get_libserialport_data(int *np) {
return sp;
}

// Free memory allocated from get_libserialport_data()
static void free_libserialport_data(SERPORT *sp, int n) {
for(int k = 0; sp && k < n; k++)
free(sp[k].sernum), free(sp[k].port);
free(sp);
}

// Returns a NULL-terminated malloc'd list of items in SERPORT list spa that are not in spb
SERPORT **sa_spa_not_spb(SERPORT *spa, int na, SERPORT *spb, int nb) {
SERPORT **ret = cfg_malloc(__func__, (na+1)*sizeof*ret);
Expand Down Expand Up @@ -266,10 +274,7 @@ int setport_from_serialadapter(char **portp, const SERIALADAPTER *sea, const cha
if(m == 0 || sa_num_matches_by_sea(sea, sernum, sp+i, 1) == 1)
sa_print_specs(sp, n, i);
}

for(int k = 0; k < n; k++)
free(sp[k].sernum), free(sp[k].port);
free(sp);
free_libserialport_data(sp, n);

return rv;
}
Expand All @@ -294,14 +299,53 @@ int setport_from_vid_pid(char **portp, int vid, int pid, const char *sernum) {
if(m == 0 || sa_num_matches_by_ids(vid, pid, sernum, sp+i, 1) == 1)
sa_print_specs(sp, n, i);
}

for(int k = 0; k < n; k++)
free(sp[k].sernum), free(sp[k].port);
free(sp);
free_libserialport_data(sp, n);

return rv;
}

// Potentially change port *portp after opening & closing it with baudrate
int touch_serialport(char **portp, int baudrate) {
int i, n1, n2;
SERPORT *sp1, *sp2, **diff;
sp1 = get_libserialport_data(&n1);
if(!sp1 || n1 <= 0 || !portp)
return -1;

pmsg_info("touching serial port %s at %d baud\n", *portp, baudrate);
union pinfo pinfo;
union filedescriptor fd;
pinfo.serialinfo.baud = baudrate;
pinfo.serialinfo.cflags = SERIAL_8N1;
if(serial_open(*portp, pinfo, &fd) == -1) {
pmsg_error("%s() failed to open port %s at %d baud\n", __func__, *portp, baudrate);
return -1;
}
serial_set_dtr_rts(&fd, 0);
serial_close(&fd);

pmsg_info("waiting for new port...");
for(i = 10; i > 0; i--) {
usleep(80*1000);
if((sp2 = get_libserialport_data(&n2))) {
diff = sa_spa_not_spb(sp2, n2, sp1, n1);
if(*diff && diff[0]->port && !diff[1]) { // Exactly one new port sprung up
pmsg_notice("new port %s discovered\n", (*diff)->port);
if(*portp)
free(*portp);
*portp = cfg_strdup(__func__, (*diff)->port);
i = -1; // Leave loop
}
free(diff);
free_libserialport_data(sp2, n2);
}
}
free_libserialport_data(sp1, n1);
msg_info(" using %s port %s\n", i<0? "new": "same", *portp);

return 0;
}

// List available serial ports
int list_available_serialports(LISTID programmers) {
// Get serial port information from libserialport
Expand Down Expand Up @@ -344,6 +388,11 @@ int list_available_serialports(LISTID programmers) {
return -1;
}

int touch_serialport(char **portp, int baudrate) {
pmsg_error("avrdude built without libserialport support; please compile again with libserialport installed\n");
return -1;
}

#endif

void list_serialadapters(FILE *fp, const char *prefix, LISTID programmers) {
Expand Down