Provide serialadapter entity in avrdude.conf

[stefanrueger]

This work-in-progress PR provides a serialadapter entity in avrdude.conf.in as a specific programmer, namely one that only defines USB parameters. In contrast to the ser-auto branch this PR intends to

• Allow inheritance of serialadapter entities, eg, you can have in your ~/.avrduderc file sth like

  serialadapter parent "ch340"
      id                     = "mysuperduperboard";
      usbsn                  = "08/15";
  ;
  

• Allow any programmer with usbpid definitions to be used as a serial adapter, eg, the ft232r programmer is also a serial adapter
• Have usbpid as a list instead of a single value
• Make all functions in serialadapter.c of the ser-auto branch superfluous as serialadapter is a programmer, so the original programmer functions will do the job
• Print serialadapter entities with developer options -c*/s

This PR also provides a SERIALADAPTER structure: actually, behind the scenes, it is the same as PROGRAMMER, but this will allow writing self-documenting code.

For the time being, this PR utilises the baudrate component of PROGRAMMER as default_baud but it would be vvv simple to have both baudrate and default_baud if needed. Here is a code example of how this PR can be utilised to retrieve a particular serial adapter (or programmer with USB definitions) from the config files:

  SERIALADAPTER *sea = locate_programmer(programmers, sea_id);
  if (sea == NULL || !is_serialadapter(sea)) {
    serialadapter_not_found(sea_id);
    exit(1);
  }

@MCUdude: Please check out this PR and see whether this is useful for what you try to achieve. I suspect @dl8dtl has reservations about viewing a serial adapter as a special unfinished programmer entry, but I am hoping that the simplicity of this PR will be somewhat persuasive.

It's possible to carry on with the ser-auto branch but adding above functionality will cost quite some work, in particular, the developer options that print AVRDUDE's understanding of its internal entities.

[MCUdude]

Thanks for the PR @stefanrueger! This approach looks much less complicated than mine, and I agree that the small tradeoffs is weighed up by the fact that the code is easier to understand, and we can utilize the developer options.

both baudrate and default_baud if needed

Allow inheritance of serialadapter entities, eg, you can have in your ~/.avrduderc file sth like

Inheritance is really neat. And I'm absolutely fine with baudrate. And I'd argue that it would make more sense to have a user-specified default baud rate in avrduderc rather than in avrdude.conf.

serialadapter parent "ch340"
    id                     = "mysuperduperboard";
    usbsn                  = "08/15";
    baudrate               = 250000;
;

Allow any programmer with usbpid definitions to be used as a serial adapter, eg, the ft232r programmer is also a serial adapter

I didn't think of this. Neat!

It's possible to carry on with the ser-auto branch but adding above functionality will cost quite some work, in particular, the developer options that print AVRDUDE's understanding of its internal entities.

IMO there is no doubt. We should continue working on this rather than my branch!

[MCUdude]

@stefanrueger where do we continue from here? From what I can read from the code, what's missing now is the libserialport part, where it's given a USB vid, pid, and an optional serial number, and a COM port number/dev path is returned.

The libserialport example code I wrote earlier this summer contains the essence of what we want to use the libserialport library for. What the example currently doesn't support is [vid]:[pid] or [vid]:[pid]:[sn] as its second argument.

[stefanrueger]

where do we continue from here?

@MCUdude Would it be OK if you built what's needed from here? Add your name as author to serialadapter.c, put your code there and modify main.c to allow users to specify -P <serialadapter|programmer>[:serialnumber] (at least that's what I guess you were thinking of). You could create a PR against the branch of this PR. Alternatively, you or @mcuee could check this PR against regression and we merge it, so you can create a PR against git main.

[MCUdude]

Perhaps we can do some basic regression testing against this PR, and then merge it as it is? I prefer to have it merged into main before continuing the work, to make my git life easier.

[stefanrueger]

prefer to have it merged into main

OK. I'll add documentation for serialadapter; when this PR has been tested against regression, I'll merge.

[dl8dtl]

I suspect @dl8dtl has reservations about viewing a serial adapter as a special unfinished programmer entry, but I am hoping that the simplicity of this PR will be somewhat persuasive.

Well, yes, I think viewing a serialadapter as just another programmer is mildly confusing. What convinces is the quality of the patch though :), in particular all the documentation updates.
I'm not sure whether there's any much value in being able to consider any programmer with a PID as a serialadapter, but maybe I'm too much used to the old Atmel tools where that really doesn't make any sense.
Regarding the future implementation, if possible, it would make sense to handle the string "usb" as a kind of pre-defined serialadapter. That would allow for unifying the current syntax that allows for e.g. … -p jtag2 -P usb:1234 (trailing serial number digits) into a generic … -p something -P serialadapter:serno one, regardless of the actual programmer type. So far, I believe that -P usb is always handled per-programmer.

[stefanrueger]

value in being able to consider any programmer with a PID as a serialadapter

There are a number of existing programmers, eg, ft232r, ft232h, ft2232h etc that you might want as serial adapters and that already have all the relevant info... There will be other programmers that are not serial adapters (by any stretch of imagination) but then allowing them is no harm as it really means I have usbvid and usbpid and want to be looked up as serial interface.

[stefanrueger]

@MCUdude @dl8dtl There is also the possibility to add either sth like IS_SERIALADAPTER to extra_features or create a boolean is_serialadapter in the programmer syntax to explicitly say which of the existing programmers double up as serial adapters, for use with the likes of ft232r. That would be an easy addition to the current state.

[MCUdude]

There is also the possibility to add either sth like IS_SERIALADAPTER to extra_features or create a boolean is_serialadapter in the programmer syntax to explicitly say which of the existing programmers double up as serial adapters, for use with the likes of ft232r.

That sounds like a good idea. Then one can look at the programmers in avrdude.conf and figure out that this ft232rl programmer can also serve as a generic serial adapter.

[stefanrueger]

@MCUdude Which programmers should this PR also treat as generic serial adapter? I assume ft232r, ft2232h, 2232hio, ft4232h, ft232h, c232hm, ft245r,ttl232r and ch341a. Some will need usbvid/usbpid entries. What else?

However, the derived avrftdi, tigard, avrisp-u, ft2232h_jtag, ft232h_jtag, um232h, tumpa, etc probably not?

Note there is no entity called ft232rl.

[MCUdude]

Which programmers should this PR also treat as generic serial adapter?

How about only the parent programmer, with a few exceptions? Then we don't fill the list with lots of serial adapter names that do the exact same thing.

The ft245 isn't a USB to serial chip, but rather a USB to parallel chip.

So I'll argue that we should add IS_SERIALADAPTER to

• ft2232h
• ft232h
• ft232r
• ft4232h
• ch341a

[MCUdude]

I tried adding the following code to main.c, just to see if I could make it print out the information from avrdude.conf. For some reason it segfaults if -P isn't ch340.

  const char *seradapter;
  SERIALADAPTER *ser = locate_programmer_set(programmers, port, &seradapter);
  if (is_serialadapter(ser))
    msg_info("port id: %s, vid: 0x%04x, pid: 0x%04x\n", (const char *)ldata(lfirst(ser->id)), ser->usbvid, *(int *)ldata(lfirst(ser->usbpid)));
  else if (is_programmer(ser))
    msg_error("programmer incorrectly specified using -P %s\n", port);
  else if (port && !seradapter)
    msg_info("Port %s specified\n", port);

But why does the above code fail when I use something else than -P ch340? Is it is_programmer and is_serialport that don't properly handle null pointers?

EDIT: I can make it not segfault by guarding against ser being NULL. But I don't see why this would be necessary due to the various checks in is_programmer and is_serialadapter.

  const char *seradapter;
  SERIALADAPTER *ser = locate_programmer_set(programmers, port, &seradapter);
  if (ser && is_serialadapter(ser))
    msg_info("port id: %s, vid: 0x%04x, pid: 0x%04x\n", (const char *)ldata(lfirst(ser->id)), ser->usbvid, *(int *)ldata(lfirst(ser->usbpid)));
  else if (ser && is_programmer(ser))
    msg_error("programmer incorrectly specified using -P %s\n", port);
  else if (port)
    msg_info("Port %s specified\n", port);

[stefanrueger]

It's a philosophical question of whether the caller or callee is supposed to check against NULL. Before using a function it's best to read its man page or, failing that, the function itself.

guarding against ser being NULL. But I don't see why this would be necessary

Try passing NULL to strlen(), str_eq(), fprintf(), ... and watch the world explode. More often than not, C-functions assume it's the caller who does the checking. Makes sense, too. You open a file once and then use the file pointer one million times to print something. So, it's expected that the caller checks once after opening and not burden the callee 1 million times with unnecessary checks against NULL.

But yes, is_programmer() and is_serialadapter() could be hardened against NULL.

[stefanrueger]

OK, implemented is_serialadapter = <yes|no> component. Now

Valid serial adapters are:
  ch340             = [usbvid 0x1a86, usbpid 0x7523]
  ch341a            = [usbvid 0x1a86, usbpid 0x5512]
  ft2232h           = [usbvid 0x0403, usbpid 0x6010]
  ft232h            = [usbvid 0x0403, usbpid 0x6014]
  ft232r            = [usbvid 0x0403, usbpid 0x6001]
  ft4232h           = [usbvid 0x0403, usbpid 0x6011]

[MCUdude]

implemented is_serialadapter = <yes|no> component.

Excellent! I think this PR is ready to be merged then

[stefanrueger]

OK, all done. @MCUdude You will need to add libserialport detection for CMakeLists.txt from your ser-auto branch (and then implement the wanted functionality).