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.

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

Add long options like --part #1936

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Add long options like --part #1936

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
96 changes: 62 additions & 34 deletions src/main.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
#include <time.h>
#include <unistd.h>
#include <ctype.h>
#include <getopt.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <sys/time.h>
Expand Down Expand Up @@ -237,39 +238,41 @@ static void usage(void) {

msg_error("Usage: %s [options]\n"
"Options:\n"
" -p <partno> Specify AVR device; -p ? lists all known parts\n"
" -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"
" -B <bitclock> Specify bit clock period (us)\n"
" -C <config-file> Specify location of configuration file\n"
" -C +<config-file> Specify additional config file, can be repeated\n"
" -N Do not load %s%s\n"
" -c <programmer> Specify programmer; -c ? and -c ?type list all\n"
" -c <wildcard>/<flags> Run developer options for matched programmers,\n"
" e.g., -c 'ur*'/s for programmer info/definition\n"
" -A Disable trailing-0xff removal for file/AVR read\n"
" -D Disable auto-erase for flash memory; implies -A\n"
" -i <delay> ISP Clock Delay [in microseconds]\n"
" -P <port> Connection; -P ?s or -P ?sa lists serial ones\n"
" -r Reconnect to -P port after \"touching\" it; wait\n"
" 400 ms for each -r; needed for some USB boards\n"
" -F Override invalid signature or initial checks\n"
" -e Perform a chip erase at the beginning\n"
" -O Perform RC oscillator calibration (see AVR053)\n"
" -t Run an interactive terminal when it is its turn\n"
" -T <terminal cmd line> Run terminal line when it is its turn\n"
" -U <memstr>:r|w|v:<filename>[:format]\n"
" Carry out memory operation when it is its turn\n"
" Multiple -t, -T and -U options can be specified\n"
" -n Do not write to the device whilst processing -U\n"
" -V Do not automatically verify during -U\n"
" -E <exitsp>[,<exitsp>] List programmer exit specifications\n"
" -x <extended_param> Pass <extended_param> to programmer, see -x help\n"
" -v Verbose output; -v -v for more\n"
" -q Quell progress output; -q -q for less\n"
" -l logfile Use logfile rather than stderr for diagnostics\n"
" -? Display this usage\n"
" -p <partno> Specify AVR device; -p ? lists all known parts\n"
" -p <wildcard>/<flags> Run developer options for matched AVR devices,\n"
" e.g., -p ATmega328P/s or /S for part definition\n"
" -b, --baud <baudrate> Override RS-232 baud rate\n"
" -B, --bitclock <bitclock> Specify bit clock period (us)\n"
" -C <config-file> Specify location of configuration file\n"
" -C +<config-file> Specify additional config file, can be repeated\n"
" -N Do not load %s%s\n"
" -c, --programmer <programmer>\n"
" Specify programmer; -c ? and -c ?type list all\n"
" -c, --programmer <wildcard>/<flags>\n"
" Run developer options for matched programmers,\n"
" e.g., -c 'ur*'/s for programmer info/definition\n"
" -A Disable trailing-0xff removal for file/AVR read\n"
" -D, --noerase Disable auto-erase for flash memory; implies -A\n"
" -i <delay> ISP Clock Delay [in microseconds]\n"
" -P, --port <port> Connection; -P ?s or -P ?sa lists serial ones\n"
" -r, --reconnect Reconnect to -P port after \"touching\" it; wait\n"
" 400 ms for each -r; needed for some USB boards\n"
" -F Override invalid signature or initial checks\n"
" -e, --erase Perform a chip erase at the beginning\n"
" -O Perform RC oscillator calibration (see AVR053)\n"
" -t, --terminal, --tty Run an interactive terminal when it is its turn\n"
" -T <terminal cmd line> Run terminal line when it is its turn\n"
" -U, --memory <memstr>:r|w|v:<filename>[:format]\n"
" Carry out memory operation when it is its turn\n"
" Multiple -t, -T and -U options can be specified\n"
" -n, --test Do not write to the device whilst processing -U\n"
" -V, --noverify Do not automatically verify during -U\n"
" -E <exitsp>[,<exitsp>] List programmer exit specifications\n"
" -x <extended_param> Pass <extended_param> to programmer, see -x help\n"
" -v, --verbose Verbose output; -v -v for more\n"
" -q Quell progress output; -q -q for less\n"
" -l, --logfile logfile Use logfile rather than stderr for diagnostics\n"
" -?, --help Display this usage\n"
"\navrdude version %s, https://github.com/avrdudes/avrdude\n",
progname, strlen(cfg) < 24? "config file ": "", cfg, AVRDUDE_FULL_VERSION);

Expand Down Expand Up @@ -814,7 +817,32 @@ int main(int argc, char *argv[]) {
#endif

// Process command line arguments
while((ch = getopt(argc, argv, "?Ab:B:c:C:DeE:Fi:l:nNp:OP:qrtT:U:vVx:")) != -1) {
struct option longopts[] = {
{"help", no_argument, NULL, '?'},
{"baud", required_argument, NULL, 'b'},
{"bitclock", required_argument, NULL, 'B'},
{"programmer", required_argument, NULL, 'c'},
{"config", required_argument, NULL, 'C'},
{"noerase", no_argument, NULL, 'D'},
{"erase", no_argument, NULL, 'e'},
{"logfile", required_argument, NULL, 'l'},
{"test", no_argument, NULL, 'n'},
Copy link
Collaborator

Choose a reason for hiding this comment

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

-n meant originally don't write to the AVR part for -U operations, where -n would write the file to stdout instead of the AVR memory. When other ways to write to the AVR were implemented (-t, -T, ...) there was no way to test these in a dryrun as it were until the -c programmer dryrun was implemented. I now wonder whether --test should be --test-U or rather --test-update to clarify the role of that option.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I still think --test is the wrong mnemonic. Given we have --memory for -U we should rename --test to --test-memory as its scope is confined to -U/--memory operations.

{"noconfig", no_argument, NULL, 'N'},
{"part", required_argument, NULL, 'p'},
{"chip", required_argument, NULL, 'p'},
{"port", required_argument, NULL, 'P'},
{"quiet", no_argument, NULL, 'q'},
{"reconnect", no_argument, NULL, 'r'},
{"terminal", no_argument, NULL, 't'},
{"tty", no_argument, NULL, 't'},
Copy link
Collaborator

Choose a reason for hiding this comment

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

Now that I look at the list carefully, I don't think tty is a good long option name to signify -t for the principal reason that tty describes a communication method (teletype), which is orthogonal to what -t does. Terminal input can come from a file, from a pipe, via a USB connection etc and not necessarily from a tty. I'd prefer to either have no second long option for -t, but if there is an appetite for a second option, I'd much rather have one of --interactive, --console, --avr-console, --shell or similar. We normally use the wording terminal or interactive terminal in the documentation for -t, so --terminal might be sufficient.

Could we add --terminal-command for -T (note capital T), which like -U is not interactive?

Copy link
Contributor Author

@ndim ndim Nov 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

"shell" implies turing completeness. "interactive" captures the spirit well.

{"memory", required_argument, NULL, 'U'},
Copy link
Collaborator

Choose a reason for hiding this comment

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

--memory is a good long option name b/c it puts the focus correctly: the operations r, w and v all refer to memory and, crucially, not the corresponding file; originally -U got its name from the generic update operations it might describe, so I would also allow the mnemonic --update or, alternatively, --update-memory. I have an ever so slight preference for the latter: personally, I think long options are good in shell scripts to improve readability, so I am less worried about the length of long options. If we go with the latter, we wouldn't need --memory.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I would argue that --memory would be sufficient. The help test will get awfully long if we have tons of long options that to the same thing. And the -U flag doesn't necessary updates the memory.

IMO, --memory flash:r:myfile.hex makes way more sense than --update-memory flash:r:myfile.hex.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Agreed, --memory is sufficient

{"verbose", no_argument, NULL, 'v'},
{"noverify", no_argument, NULL, 'V'},
{NULL, 0, NULL, 0}
};
while((ch = getopt_long(argc, argv,
"?Ab:B:c:C:DeE:Fi:l:nNp:OP:qrtT:U:vVx:",
longopts, NULL)) != -1) {
switch(ch) {
case 'b': // Override default programmer baud rate
baudrate = str_int(optarg, STR_INT32, &errstr);
Expand Down
Loading