Re: Introducing SerialUPDI programmer

[Dawid Buchwald]

Hey,

To clarify the reason why I need to change serial code - it’s not about the 
speeds (although I had to add 300/600 baud, but that’s a different story), it’s 
about the transmission parameters.

See, all the other serial programmers use 8 bit per byte, no parity and single 
stop bit - and this setup was hardcoded in ser_posix.c and ser_win32.c, 
respectively:

termios.c_cflag = (CS8 | CREAD | CLOCAL);

dcb.ByteSize = 8;
dcb.Parity = NOPARITY;
dcb.StopBits = ONESTOPBIT;

Now for UPDI it uses different settings - even parity enabled and two stop bits 
(snippet below is coming from pymcuprog utility by Microchip):

self.ser = serial.Serial(port, baud, parity=serial.PARITY_EVEN, timeout=1, 
stopbits=serial.STOPBITS_TWO)

So this is why I had to change the ser_setspeed function - it needs to take new 
parameters and translate them to system-specific values. It also required 
change to all the serial-based programmers, so they would pass the default 
value.

As for patch management I would suggest to process this one as a separate 
patch. I might be wrong, but I would rather work with simple, smaller and 
easier to manage incremental changes. I have created 
https://savannah.nongnu.org/patch/index.php?10154 for this.

If you prefer to receive all in one big package, you can reject this patch.

Best regards,
Dawid Buchwald


On 30 Nov 2021, at 21:02, Joerg Wunsch 
<j@uriah.heep.sax.de<mailto:j@uriah.heep.sax.de>> wrote:

As Dawid Buchwald wrote:

Anyway, as for the implementation, I’m still working on it, and
probably will, at least for a while. What I do need to know right
now, is whether it is OK to change some of the core AVRDUDE serial
code. The thing is that for this particular implementation I need to
be able to change serial connection parameters - UPDI uses slightly
different ones. What I did was change to union pinfo, like so:

It's certainly OK to change things if needed.

And, obviously, changed each and every occurrence to set the values to default, 
like so:

pinfo.serialinfo.baud = 19200;
pinfo.serialinfo.cflags = SERIAL_DEFAULT_FLAGS;

Well, the biggest issue here is, that entire baudrate switching code
is there since the old JTAGICEmkII uses two different baud rates: it
attaches with 19200 Bd, but after initialization switches to 115200
Bd.

XBee has another hack to decide between two different baud rates, but
only during startup, not back and forth as with the JTAGICEmkII.

For all other devices, if a baudrate is needed at all, it's either
specified by the user on the command-line, or a default from the "pgm"
object is used.

Not sure what your cflags is about though. Is that the termios.c_cflag
value? It can probably be handled there if needed.

Bonus question: should it all go in one huge patch, or should there
be one first, containing only change enabling different serial
connection parameters, followed later on by the one related to
SerialUPDI implementation only? Any preference?

It would be cool if you sketch out some of the implementation details
here in the mailinglist before coding everything.

Otherwise, use good judgement for what belongs into a single patch,
and what would better be separated.

--
cheers, Joerg               .-.-.   --... ...--   -.. .  DL8DTL

https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.sax.de%2F~joerg%2F&amp;data=04%7C01%7C%7C89a53c3f9f714118fc7608d9b43cadd0%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637738994968220054%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=ORvJ4KrIIUDg6KO33Tzs4QiHvkPkisjXE7KIGFy50QI%3D&amp;reserved=0
Never trust an operating system you don't have sources for. ;-)