avrdudes / avrdude

AVRDUDE is a utility to program AVR microcontrollers
GNU General Public License v2.0
756 stars 139 forks source link

Support options like `--config` `--part` and `--port` #1922

Open MCUdude opened 2 months ago

MCUdude commented 2 months ago

I created a post over at the Avrfreaks forum announcing that Avrdude v8.0 has just been released. The feedback was mixed (some pointing out that Avrdude is just a CLI tool and not a full-blown GUI, and thus not relevant in 2024), but @westfw mentioned that the option flags (-C, -c, -p etc) could be difficult to remember, and having full-blown --flags would be nice.

Avrdude currently uses getopt, and this doesn't support --flags. Instead, we'll have to use argp.

Are there any downsides to using agrp instead of getopt?

ndim commented 2 months ago

In PR #1698, I have replaced the getopt call with getopt_long in order to add --version.

While at it, I also added--help.

Quoting the FreeBSD man page for getopt_long:

HISTORY
     The getopt_long() and getopt_long_only() functions first appeared in the
     GNU libiberty library.  The first BSD implementation of getopt_long()
     appeared in NetBSD 1.5, the first BSD implementation of
     getopt_long_only() in OpenBSD 3.3.  FreeBSD first included getopt_long()
     in FreeBSD 5.0, getopt_long_only() in FreeBSD 5.2.

We already have a Windows implementation of getopt_long inside src/msvc/getopt.c, so changing from getopt to getopt_long is very quick.

So getopt_long is covered on all systems with glibc i.e. Linux and compatible (e.g. musl), BSD (and therefore MacOS), and Windows.

The argp API is a lot more powerful, of course, but it appears avrdude would need to ship an argp implementation for all non-glibc platforms in order to use the argp API with its advantages like autogenerated --help output.

ndim commented 2 months ago

To actually answer the question: The downside to using argp is the work needed to ensure its availability on all systems other than Linux/glibc, both from a technical and from a licensing point of view.

WestfW commented 2 months ago

getopt_long() seems to drop in pretty painlessly, and is already present in the windows getopt used by avrdude, and works on both macos and linux builds as well, so it should be pretty safe.

(from the avrfreaks discussion): I replaced in implicit include for getopt() from unistd.h with an explicit include of getopt.h (which is already present in src/msvc/)

Then it was just a matter of replacing the getopt() call with one to getopt_long() (and adding the structure with the long form commands...)

Everything else stayed the same!

 // Process command line arguments
 static struct option long_options[] =
   {
     { "baud",      required_argument, 0, 'b' },
     { "bitclock",  required_argument, 0, 'B'},
     { "programmer", required_argument, 0, 'c'},
     { "config",    required_argument, 0, 'C'},
     { "noerase",   no_argument,      0, 'D'},
     { "erase",     no_argument,      0, 'e'},
     { "logfile",   required_argument, 0, 'l'},
     { "test",      no_argument,      0, 'n'},
     { "noconfig",  no_argument,      0, 'N'},
     { "part",      required_argument, 0, 'p'},
     { "chip",      required_argument, 0, 'p'},
     { "port",      required_argument, 0, 'P'},
     { "quiet",     no_argument,      0, 'q'},
     { "reconnect", no_argument,      0, 'r'},
     { "terminal",  no_argument,      0, 't'},
     { "tty",       no_argument,      0, 't'},
     { "memory",    required_argument, 0, 'U'},
     { "verbose",   no_argument,      0, 'v'},
     { "noverify",  no_argument,      0, 'V'},
     {0, 0, 0, 0}
   };
 int long_index;

 while((ch = getopt_long(argc, argv,
                         "?Ab:B:c:C:DeE:Fi:l:nNp:OP:qrtT:U:vVx:",
                         long_options, &long_index)) != -1) {
   switch(ch) {

That does leave a bunch of help text and error messages that contain only the short command forms...

stefanrueger commented 2 weeks ago

@ndim Thanks for above comment about getopt_long(). I assume that supersedes your previous comment expressing getopt_long() might not be portable to all OSes we are currently supporting? If you think we'd be OK with getopt_long() and @WestfW's table above would you be interested in extending --version and --help along above table? That would be cool. The main outstanding work will be updating help texts in main.c and updating the man and tex documentation.

ndim commented 6 days ago

Both --part and --chip mapping to p makes sense to me, but @WestfW your long_options struct contains two long options terminal and tty which both map to the same character t. Is that on purpose?

stefanrueger commented 4 days ago

I am guessing(!) tty ought to map tp P. Also, on closer reflextion, perhaps increase coverage, ie, invent a long version for every option letter?

ndim commented 4 days ago

FWIW, I have examined the argp implementation inside gnulib with a view towards possible redistribution inside the avrdude repo, and for linking into the avrdude command line tool. The argp features would be very nice to use, after all.

The gnulib argp implementation appears to be licensed LGPL-3.0-or-later and GPL-3.0-or-later and originate from the GNU libc. There are already two GPL-3 (without "or-later") source files in the avrdude repo (avrintel.c and libavrdude-avrintel.h) alongside many GPL-2.0-or-later files, so linking GPL-3.0-or-later sources into libavrdude or avrdude would not change their respective binary's (library or CLI tool) GPL-3.0 terms, AFAICT.

[user@home ~/avrdude]$ tabs -30
[user@home ~/avrdude]$ licensecheck --machine --shortname-scheme=spdx src/*.[ch]
src/arduino.c                 GPL-2.0-or-later
src/arduino.h                 GPL-2.0-or-later
src/avr910.c                  GPL-2.0-or-later
...
src/whereami.h                WTFPL-2
src/wiring.c                  GPL-2.0-or-later
src/wiring.h                  GPL-2.0-or-later
src/xbee.c                    GPL-2.0-or-later
src/xbee.h                    GPL-2.0-or-later
[user@home ~/avrdude]$ tabs -8

I have not yet verified that the gnulib argp implementation would actually build for and run on all of avrdude's supported operating systems. If avrdude were to switch to argp, glibc on Linux systems would obviously work, but non-glibc Linux systems, the BSDs, MacOS, and Windows still need to be examined.

@stefanrueger Is the GPL-3.0 only licensing for avrintel.c and libavrdude-avrintel.h on purpose? If yes, the repo should ship that license text, as the built libavrdude and avrdude will then fall under GPL-3.0, not the GPL-2.0-or-later which the COPYING file describes and most *.[ch] files use.

If the move to GPL-3.0 is on purpose, I can check whether we could use gnulib's argp implementation.

If we can relicense the avrintel.c and libavrdude-avrintel.h files to something compatible with GPL-2.0-or-later and avrdude and libavrdude is supposed to remain GPL-2.0-or-later, then continuing to invest time into argp would not make sense, as argp's LGPL-3.0-or-later together with avrdude's GPL-2.0-or-later would result in a GPL-3.0-or-later binary.

stefanrueger commented 4 days ago

Is the GPL-3.0 only licensing for avrintel.c and libavrdude-avrintel.h on purpose?

No: I've just used a licence that I was using for other projects. I am happy to put another licence on these files that fits avrdude better, so happy to change to GPL-2.0-or-later.

Wrt to argp I'd argue that this decision should be more driven by how much we need its features. I tend to go with a philosophy of using what we cannot do without rather than using what we could do with. And yes, if we don't really need argp's unique features then it's better to keep avrdude at gpl-2.0-or-later as (in theory) more projects that interested to use avrdude will be able to cope with that.

MCUdude commented 3 days ago

I think getopt_long would be sufficient. The only place where support for "multiple words" would be neat would be with the -U flag.

However, it would be a big improvement if it would be possible to support something like this, and print a sensible error message if the user got the -U / --memory syntax wrong:

-U flash:write:myfile.hex
-U flash:read:mydump.hex
-U flash:verify:myfile.hex

--memory flash:write:myfile.hex
--memory flash:read:mydump.hex
--memory flash:verify:myfile.hex

--memory flash:w:myfile.hex
--memory flash:r:mydump.hex
--memory flash:v:myfile.hex
ndim commented 3 days ago

Just had an idea: It might be possible to find a LGPL-2.0-or-later argp implementation in older versions of gnulib.

Here we have the lib/argp*.[ch] files as GPL-2.0-or-later

Source repo: git://git.savannah.gnu.org/gnulib.git
git hash:    b42d50108cf2147b3d80ecb6457faa96347411e9
Date:        Sun Oct 7 17:38:16 2007 +0200

but making them compile without copying successively more files from gnulib is non-trivial (and I do not know where this would end). Also, there might have been important fixes between 2007-10-07 and now...

Summarizing: For avrdude, I do prefer getopt_long over argp_parse now.

stefanrueger commented 3 days ago

Extending the -U syntax to allow flash:write:myfile.hex might be somewhat involved (currently there are only one-letter memory operations r, w and v foreseen in the syntax). Also sensible error messages might be hard to come by as : is both an admissible file character in Windows and a syntax separator for -U, so avrdude can never be quite sure which form of memory command was intended if it is somehow malformed. There is

It helps that op and encoding are only ever one character.

So to cut a long short, I recommend keeping this issue at the scope of long options and documenting them.