Skip to content

Extended parameters (-x and -E) improvements #1844

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.

Sign up for GitHub

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

Merged
merged 21 commits into from
Jul 31, 2024
Merged

Extended parameters (-x and -E) improvements #1844

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
aebf970
Add -xautoreset option for the AVR109 programmer
MCUdude Jul 22, 2024
5e4a9a7
Add -xno_autoreset for the -c arduino programmer
MCUdude Jul 22, 2024
0d7524d
Add -xno_autoreset for the -c wiring programmer
MCUdude Jul 22, 2024
c3d97e9
Add -xnoautoreset for the -c urclock programmer
stefanrueger Jul 28, 2024
94e968a
Use %i instead of %2d
MCUdude Jul 28, 2024
12c41f2
Small tweaks based on feedback from @stefanrueger
MCUdude Jul 28, 2024
cda52a3
Fix autoreset flag for -c arduino
MCUdude Jul 28, 2024
c1a0378
Print -x help text when invalid extended option is passed
MCUdude Jul 29, 2024
568bb8e
Use bool instead of int for help flag
MCUdude Jul 29, 2024
313c8b4
Print helptext for all programmers that supports extended params when…
MCUdude Jul 29, 2024
f37e593
Clarify invalid value messages for valid -x parameters ...
stefanrueger Jul 29, 2024
df5d1fe
Add remaining spaces after -x in messages
stefanrueger Jul 29, 2024
41e7d81
Fix issue where help flag wasn't set when it should
MCUdude Jul 29, 2024
9658908
check if pgm->type is Arduino instead of pgmid
MCUdude Jul 29, 2024
95b4e78
Improve -x help for the remaining programmers
MCUdude Jul 29, 2024
3c679b1
Add help text for -E exitspec
MCUdude Jul 29, 2024
297d256
Fix incorrect noautoreset implementation for the Wiring programmer op…
MCUdude Jul 29, 2024
38da070
Fix bug in the serprog ext parameters parsing
MCUdude Jul 29, 2024
77db93b
Fix formatting and missing brackets
MCUdude Jul 29, 2024
a874b52
help flag should be false by default
MCUdude Jul 29, 2024
92a2729
help flag should be false by default, again
MCUdude Jul 29, 2024
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
88 changes: 65 additions & 23 deletions src/arduino.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,45 @@
#include "stk500.h"
#include "arduino.h"

static int arduino_parseextparms(const PROGRAMMER *pgm, const LISTID extparms) {
const char *extended_param;
int attempts;
int rv = 0;

for (LNODEID ln = lfirst(extparms); ln; ln = lnext(ln)) {
extended_param = ldata(ln);

if (sscanf(extended_param, "attempts=%2d", &attempts) == 1) {
PDATA(pgm)->retry_attempts = attempts;
pmsg_info("setting number of retry attempts to %d\n", attempts);
continue;
}

if(str_starts(extended_param, "no_autoreset")) {
if(!str_eq(extended_param, "no_autoreset")) {
pmsg_error("invalid -xno_autoreset value %s. Use -xno_autoreset\n", extended_param);
rv = -1;
break;
}
PDATA(pgm)->autoreset = false;
continue;
}

if (str_eq(extended_param, "help")) {
msg_error("%s -c %s extended options:\n", progname, pgmid);
msg_error(" -xattempts=<arg> Specify no. connection retry attempts\n");
msg_error(" -xno_autoreset Don't toggle RTS/DTR lines on port open to prevent a hardware reset\n");
msg_error(" -xhelp Show this help menu and exit\n");
return LIBAVRDUDE_EXIT;
}

pmsg_error("invalid extended parameter '%s'\n", extended_param);
rv = -1;
}

return rv;
}

/* read signature bytes - arduino version */
static int arduino_read_sig_bytes(const PROGRAMMER *pgm, const AVRPART *p, const AVRMEM *m) {
unsigned char buf[32];
Expand Down Expand Up @@ -85,28 +124,28 @@ static int arduino_open(PROGRAMMER *pgm, const char *port) {
return -1;
}

// This code assumes a negative-logic USB to TTL serial adapter
// Set RTS/DTR high to discharge the series-capacitor, if present
serial_set_dtr_rts(&pgm->fd, 0);
/*
* Long wait needed for optiboot: otherwise the second of two bootloader
* calls in quick succession fails:
*
* avrdude -c arduino -qqp m328p -U x.hex; avrdude -c arduino -qqp m328p -U x.hex
*/
usleep(250 * 1000);
// Pull the RTS/DTR line low to reset AVR
serial_set_dtr_rts(&pgm->fd, 1);
// Max 100 us: charging a cap longer creates a high reset spike above Vcc
usleep(100);
// Set the RTS/DTR line back to high, so direct connection to reset works
serial_set_dtr_rts(&pgm->fd, 0);

usleep(100 * 1000);

/*
* drain any extraneous input
*/
if(PDATA(pgm)->autoreset) {
// This code assumes a negative-logic USB to TTL serial adapter
// Set RTS/DTR high to discharge the series-capacitor, if present
serial_set_dtr_rts(&pgm->fd, 0);
/*
* Long wait needed for optiboot: otherwise the second of two bootloader
* calls in quick succession fails:
*
* avrdude -c arduino -qqp m328p -U x.hex; avrdude -c arduino -qqp m328p -U x.hex
*/
usleep(250 * 1000);
// Pull the RTS/DTR line low to reset AVR
serial_set_dtr_rts(&pgm->fd, 1);
// Max 100 us: charging a cap longer creates a high reset spike above Vcc
usleep(100);
// Set the RTS/DTR line back to high, so direct connection to reset works
serial_set_dtr_rts(&pgm->fd, 0);

usleep(100 * 1000);
}

// Drain any extraneous input
stk500_drain(pgm, 0);

if (stk500_getsync(pgm) < 0)
Expand All @@ -115,7 +154,7 @@ static int arduino_open(PROGRAMMER *pgm, const char *port) {
return 0;
}

static void arduino_close(PROGRAMMER * pgm)
static void arduino_close(PROGRAMMER *pgm)
{
serial_close(&pgm->fd);
pgm->fd.ifd = -1;
Expand All @@ -131,9 +170,12 @@ void arduino_initpgm(PROGRAMMER *pgm) {
stk500_initpgm(pgm);

strcpy(pgm->type, "Arduino");
PDATA(&pgm)->autoreset = true; // Auto-reset enabled by default
Copy link
Collaborator

@stefanrueger stefanrueger Jul 28, 2024

Choose a reason for hiding this comment

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

This looks wrong (because it probably is): Must be PDATA(pgm) not PDATA(&pgm).I think this sets an unknown memory address to 1 (but not autoreset).

Copy link
Collaborator Author

@MCUdude MCUdude Jul 28, 2024

Choose a reason for hiding this comment

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

It works this way, and I got a segfault if not. I'll look into it again

Copy link
Collaborator

@stefanrueger stefanrueger Jul 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I think I know what happens: arduino_initpgm() is called before pgm->setup() which allocates the pdata structure. So, pgm->cookie is NULL here (as it hasn't been initialised yet) and you get a seg-fault when you access it. PDATA(&pgm)->autorest is a memory location in the PROGRAMMER sructure which is at the offset of autoreset from where the cookie pointer lives. This is defo not what you want! Either set PDATA(pgm)->autoreset in pgm->setup() or later but before the parameters are parsed. Or invert the logic: Use a variable noautoreset that gets automatically initialised to zero. So you don't need to initialise that.


pgm->read_sig_bytes = arduino_read_sig_bytes;
pgm->open = arduino_open;
pgm->close = arduino_close;
pgm->parseextparams = arduino_parseextparms;

cx->avr_disableffopt = 1; // Disable trailing 0xff removal
}
58 changes: 51 additions & 7 deletions src/butterfly.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ struct pdata
int ctype; // Cache one byte for flash
unsigned char cvalue;
unsigned long caddr;
bool autoreset;
};

#define PDATA(pgm) ((struct pdata *)(pgm->cookie))
Expand Down Expand Up @@ -361,9 +362,8 @@ static void butterfly_enable(PROGRAMMER *pgm, const AVRPART *p) {
static int butterfly_open(PROGRAMMER *pgm, const char *port) {
union pinfo pinfo;
pgm->port = port;
/*
* If baudrate was not specified use 19200 Baud
*/

// If baudrate was not specified use 19200 Baud
if(pgm->baudrate == 0) {
pgm->baudrate = 19200;
}
Expand All @@ -373,9 +373,22 @@ static int butterfly_open(PROGRAMMER *pgm, const char *port) {
return -1;
}

/*
* drain any extraneous input
*/
if(PDATA(pgm)->autoreset) {
// This code assumes a negative-logic USB to TTL serial adapter
// Set RTS/DTR high to discharge the series-capacitor, if present
imsg_notice2("Toggling the DTR/RTS lines to trigger a hardware reset\n");
serial_set_dtr_rts(&pgm->fd, 0);
usleep(250 * 1000);
// Pull the RTS/DTR line low to reset AVR
serial_set_dtr_rts(&pgm->fd, 1);
// Max 100 us: charging a cap longer creates a high reset spike above Vcc
usleep(100);
// Set the RTS/DTR line back to high, so direct connection to reset works
serial_set_dtr_rts(&pgm->fd, 0);
usleep(100 * 1000);
}

// Drain any extraneous input
(void) butterfly_drain(pgm, 0);

return 0;
Expand Down Expand Up @@ -681,6 +694,37 @@ static int butterfly_read_sig_bytes(const PROGRAMMER *pgm, const AVRPART *p, con
return 3;
}

static int butterfly_parseextparms(const PROGRAMMER *pgm, const LISTID extparms) {
const char *extended_param;
int rv = 0;

for (LNODEID ln = lfirst(extparms); ln; ln = lnext(ln)) {
extended_param = ldata(ln);

if(str_starts(extended_param, "autoreset")) {
if(!str_eq(extended_param, "autoreset")) {
pmsg_error("invalid -xautoreset value %s. Use -xautoreset\n", extended_param);
rv = -1;
break;
}
PDATA(pgm)->autoreset = true;
continue;
}

if (str_eq(extended_param, "help")) {
msg_error("%s -c %s extended options:\n", progname, pgmid);
msg_error(" -xautoreset Toggle RTS/DTR lines on port open to issue a hardware reset\n");
msg_error(" -xhelp Show this help menu and exit\n");
exit(0);
}

pmsg_error("invalid extended parameter '%s'\n", extended_param);
rv = -1;
}

return rv;
}

const char butterfly_desc[] = "Atmel Butterfly evaluation board; Atmel AppNotes AVR109, AVR911";

void butterfly_initpgm(PROGRAMMER *pgm) {
Expand Down Expand Up @@ -714,7 +758,7 @@ void butterfly_initpgm(PROGRAMMER *pgm) {
pgm->paged_load = butterfly_paged_load;

pgm->read_sig_bytes = butterfly_read_sig_bytes;

pgm->parseextparams = butterfly_parseextparms;
pgm->setup = butterfly_setup;
pgm->teardown = butterfly_teardown;
pgm->flag = 0;
Expand Down
2 changes: 1 addition & 1 deletion src/stk500.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ int stk500_getsync(const PROGRAMMER *pgm) {

for (attempt = 0; attempt < max_sync_attempts; attempt++) {
// Restart Arduino bootloader for every sync attempt
if (str_eq(pgm->type, "Arduino") && attempt > 0) {
if (PDATA(pgm)->autoreset && attempt > 0) {
// This code assumes a negative-logic USB to TTL serial adapter
// Pull the RTS/DTR line low to reset AVR: it is still high from open()/last attempt
serial_set_dtr_rts(&pgm->fd, 1);
Expand Down
3 changes: 3 additions & 0 deletions src/stk500.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,9 @@ struct pdata {
double fosc_data;

unsigned xtal; // Set STK500 XTAL frequency

// Flag to enable/disable autoreset for the arduino programmer
bool autoreset;
};

#define PDATA(pgm) ((struct pdata *)(pgm->cookie))
Expand Down
30 changes: 21 additions & 9 deletions src/wiring.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
*/
struct wiringpdata {
int snoozetime, delay;
bool autoreset;
};


Expand Down Expand Up @@ -106,10 +107,21 @@ static int wiring_parseextparms(const PROGRAMMER *pgm, const LISTID extparms) {
pmsg_notice2("%s(): delay set to %d ms\n", __func__, val);
WIRINGPDATA(pgm)->delay = val;
continue;
} else if (str_eq(extended_param, "help")) {
}
else if(str_starts(extended_param, "no_autoreset")) {
if(!str_eq(extended_param, "no_autoreset")) {
pmsg_error("invalid -xno_autoreset value %s. Use -xno_autoreset\n", extended_param);
rv = -1;
break;
}
WIRINGPDATA(pgm)->autoreset = false;
continue;
}
else if (str_eq(extended_param, "help")) {
msg_error("%s -c %s extended options:\n", progname, pgmid);
msg_error(" -xsnooze=<arg> Wait snooze [ms] before protocol sync after port open\n");
msg_error(" -xdelay=<arg> Add delay [ms] after reset, can be negative\n");
msg_error(" -xno_autoreset Don't toggle RTS/DTR lines on port open to prevent a hardware reset\n");
msg_error(" -xhelp Show this help menu and exit\n");
return LIBAVRDUDE_EXIT;;
}
Expand All @@ -134,19 +146,19 @@ static int wiring_open(PROGRAMMER *pgm, const char *port) {
if (WIRINGPDATA(pgm)->snoozetime > 0) {
timetosnooze = WIRINGPDATA(pgm)->snoozetime;

pmsg_notice2("wiring_open(): snoozing for %d ms\n", timetosnooze);
pmsg_notice2("%s(): snoozing for %d ms\n", __func__, timetosnooze);
while (timetosnooze--)
usleep(1000);
pmsg_notice2("wiring_open(): done snoozing\n");
} else {
pmsg_notice2("%s(): done snoozing\n", __func__);
} else if (WIRINGPDATA(pgm)->autoreset) {
// This code assumes a negative-logic USB to TTL serial adapter
// Set RTS/DTR high to discharge the series-capacitor, if present
pmsg_notice2("wiring_open(): releasing DTR/RTS\n");
pmsg_notice2("%s(): releasing DTR/RTS\n", __func__);
serial_set_dtr_rts(&pgm->fd, 0);
usleep(50*1000);

// Pull the RTS/DTR line low to reset AVR
pmsg_notice2("wiring_open(): asserting DTR/RTS\n");
pmsg_notice2("%s(): asserting DTR/RTS\n", __func__);
serial_set_dtr_rts(&pgm->fd, 1);

// Max 100 us: charging a cap longer creates a high reset spike above Vcc
Expand All @@ -170,8 +182,7 @@ static int wiring_open(PROGRAMMER *pgm, const char *port) {
return 0;
}

static void wiring_close(PROGRAMMER * pgm)
{
static void wiring_close(PROGRAMMER *pgm) {
serial_close(&pgm->fd);
pgm->fd.ifd = -1;
}
Expand All @@ -184,9 +195,10 @@ void wiring_initpgm(PROGRAMMER *pgm) {
stk500v2_initpgm(pgm);

strcpy(pgm->type, "Wiring");
WIRINGPDATA(&pgm)->autoreset = true; // Auto-reset enabled by default

pgm->open = wiring_open;
pgm->close = wiring_close;

pgm->setup = wiring_setup;
pgm->teardown = wiring_teardown;
pgm->parseextparams = wiring_parseextparms;
Expand Down