avrdudes / avrdude

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

Cannot use Windows drive letters with -U option #1325

Closed mariusgreuel closed 1 year ago

mariusgreuel commented 1 year ago

Not sure if this has been reported previously: The command-line parser seems to confuse d: with r|w|v:. Possible fix is to parse :\ as one token.

D:\Work\avrdudes\avrdude\out\build\x64-Debug\src>avrdude -c atmelice -p m32u4 -U flash:w:D:\Work\avrdudes\avrdude_test\arduino_micro_blink_slow.hex
avrdude error: invalid file format '\Work\avrdudes\avrdude_test\arduino_micro_blink_slow.hex' in update specifier
avrdude error: unable to parse update operation 'flash:w:D:\Work\avrdudes\avrdude_test\arduino_micro_blink_slow.hex'
dl8dtl commented 1 year ago

It is documented that, when using CP/M, ähem, sorry, Windows "drive" letters, you have to specify all elements of the -U expression – including the final ":i" in your case (for the hex file). Without the full set of colons, the parser cannot resolve the ambiguity between a drive letter delimiter and the other delimiting colons.

(Well, those of us who had to swap floppies on CP/M computers know why there used to be drive letters. In 21st century, they look like a real anachronism to me.)

stefanrueger commented 1 year ago

Confirmed

$ mv blink.hex C:blink.hex
$ avrdude -qqU flash:w:C:blink.hex:i && echo OK
OK

So can use colon in filename! :wink:

mariusgreuel commented 1 year ago

you have to specify all elements of the -U expression

Hm, I may have read that some time ago. For many years, I used to append :i automatically. Last year I started to wonder why I did not have AVRDUDE determine the file format automatically, so I dropped the :i. Now I know why I started out using :i :-).

Anyway, let's keep this PR open. Although I have not looked at the parser, I think I can defuse this issue for Windows users. I am assigning this issue to me.

stefanrueger commented 1 year ago

defuse this issue for Windows users

It's not only windows, but could also affect other OSes with files that happen to include colons (see my example above). The suggested parsing of :\ as one token is not a great strategy

One (actually, a usual) strategy for demoting special characters is by escaping them with another or the same special character (let's say \ here) which in turn necessitates to be able to escape that character too, so that if your file is called :\:\\: you'd need to type \:\\\:\\\\\: which in turn (should you use \ as escape character) would need an unquoted shell string to double up to \\:\\\\\\:\\\\\\\\\\:. Doable.

I kinda like the simplicity of specifying all 4 elements to disambiguate file names with colons.

dl8dtl commented 1 year ago

:-)

It reminds me to some "sed" usages, where you have to escape slashes, and end up with \/\/ sequences … "Keilschrift".

mariusgreuel commented 1 year ago

but could also affect other OSes

I was planning to wrap the code with a #ifdef WIN32

Even under windows there are colon-filenames

Yes, but this syntax is a rare exception that has no practical use. The far more common case is a full path, passed via a script or dropped into the prompt.

A file might legitimately start with \

The #ifdef takes care of this

One (actually, a usual) strategy for demoting special characters is by escaping them with another or the same special character

I cannot really tell whether this is an early April Fools' joke or something you would consider ;-)

stefanrueger commented 1 year ago

[X:blink.hex] rare exception that has no practical use

Isn't it the case that each drive X has a current directory associated with it that can be addressed in short by X:?

early April Fools' joke or something you would consider

Defo the latter - there is actually code in avrdude to escape and unescape C-type strings

passed via a script

Well, couldn't the script append the :i, which must take all of 5 µs? Both to code and to execute? Or escape the string as necessary?

Another way of handling ambiguity is to consider the few possible interpretations and admit the "most likely" one: If -U flash:w:C:blink.hex can't open C as file and fails to recognise the file type blink.hex , but another interpretation manages to open the file "C:blink.hex" then take the latter. Is conceptually easier to understand for the user, but (depending how zealous you seek out combinations) might be much harder to program than escaping/unescaping strings. You get into hot water in -U flash:w:C:i when both C is a perfectly valid Intel hex file and C:i a perfectly formed .elf file...

stefanrueger commented 1 year ago

end up with \/\/ sequences

Wasn't the day when regexp got an option allowing internal comments and whitespace to clarify them a mild disappointment for all of us who just loved expressions such as /^[+-]?([0-9]{1,3}(,?[0-9]{3})*(\.[0-9]+)?|\.[0-9]+)%? ?- ?[+-]?([0-9]{1,3}(,?[0-9]{3})*(\.[0-9]+)?|\.[0-9]+)%?$/?

stefanrueger commented 1 year ago

Another, perhaps less cumbersome, idea is to try two separators : or / and go with what makes the cut. Wouldn't #ifdef that for consistency across OSes (and backwards compatibility). So can do -U C:blink.hex: that would work automagically after the second separator / has been tried (no separator? must be a file) and the first : failed (C means the calibration memory, but blink.hex is no valid r/w/v mode).

dl8dtl commented 1 year ago

Well, given that these long path names are mostly used in scripts / Makefiles and such, I think denoting them with all four fields is still the easiest way. Note that the file format could even be specified as :a for "automatic".

The abbreviated forms (up to just -U filename) have been mainly added to the parser to make it easier to manually type these command lines.

mcuee commented 1 year ago

Well, given that these long path names are mostly used in scripts / Makefiles and such, I think denoting them with all four fields is still the easiest way. Note that the file format could even be specified as :a for "automatic".

I am getting used to the append :i automatically so I think it is not a real issue to me. Occasionally I will feel that the command line from Arduino is really long but at least it runs without issues under Windows.

The abbreviated forms (up to just -U filename) have been mainly added to the parser to make it easier to manually type these command lines.

Actually I like the abbreviated forms like -U. In fact, I would like to have another one for reading of flash memory (say, -R).

I have some ideas for potential improvement for the command line options in a new issue.

mcuee commented 1 year ago

Confirmed

$ mv blink.hex C:blink.hex
$ avrdude -qqU flash:w:C:blink.hex:i && echo OK
OK

So can use colon in filename! 😉

I feel using colon is really a bad idea as it confuses people. The colon is supposed to be the separator for the four fields.

mcuee commented 1 year ago

As mentioned, I am getting used to the append :i automatically if I use -U flash:w:file.hex:i so I think it is not a real issue to me.

Actually common solution is to require the whole path to be wrapped into a pair of quotation mark like "full path" for such case under Windows, especially when there is a space in the path name, which is much easier than using escape.

1) To me it will be good that we can fix the following with plain -U.

C:\work\avr\avrdude_test\avrdude_bin> .\avrdude -c usbasp -p m328p -U "C:\work\avr\avrdude_test\avrdude_bin\test hex files\blink_uno_with_bootloader.hex"
avrdude error: invalid I/O mode '\' in update specification
  allowed values are:
    r = read device
    w = write device
    v = verify device
avrdude error: unable to parse update operation 'C:\work\avr\avrdude_test\avrdude_bin\test hex files\blink_uno_with_bootloader.hex'

2) This is for sure not working, not a real problem.

C:\work\avr\avrdude_test\avrdude_bin> .\avrdude -c usbasp -p m328p -U flash:w:C:\work\avr\avrdude_test\avrdude_bin\test hex files\blink_uno_with_bootloader.hex:i
avrdude error: invalid file format '\work\avr\avrdude_test\avrdude_bin\test' in update specifier
avrdude error: unable to parse update operation 'flash:w:C:\work\avr\avrdude_test\avrdude_bin\test'

3) At least the following is working. That is good.

C:\work\avr\avrdude_test\avrdude_bin> .\avrdude -c usbasp -p m328p -U flash:w:"C:\work\avr\avrdude_test\avrdude_bin\test hex files\blink_uno_with_bootloader.hex":i

avrdude: AVR device initialized and ready to accept instructions
avrdude: device signature = 0x1e950f (probably m328p)
avrdude: Note: flash memory has been specified, an erase cycle will be performed.
         To disable this feature, specify the -D option.
avrdude: erasing chip
avrdude: reading input file C:\work\avr\avrdude_test\avrdude_bin\test hex files\blink_uno_with_bootloader.hex for flash
         with 1426 bytes in 3 sections within [0, 0x7fff]
         using 12 pages and 110 pad bytes
avrdude: writing 1426 bytes flash ...

Writing | ################################################## | 100% 0.37 s

avrdude: 1426 bytes of flash written
avrdude: verifying flash memory against C:\work\avr\avrdude_test\avrdude_bin\test hex files\blink_uno_with_bootloader.hex

Reading | ################################################## | 100% 0.20 s

avrdude: 1426 bytes of flash verified

avrdude done.  Thank you.
mcuee commented 1 year ago

Actually common solution is to require the whole path to be wrapped into a pair of quotation mark like "full path" for such case under Windows, especially when there is a space in the path name, which is much easier than using escape.

  1. To me it will be good that we can fix the following with plain -U.
C:\work\avr\avrdude_test\avrdude_bin> .\avrdude -c usbasp -p m328p -U "C:\work\avr\avrdude_test\avrdude_bin\test hex files\blink_uno_with_bootloader.hex"
avrdude error: invalid I/O mode '\' in update specification
  allowed values are:
    r = read device
    w = write device
    v = verify device
avrdude error: unable to parse update operation 'C:\work\avr\avrdude_test\avrdude_bin\test hex files\blink_uno_with_bootloader.hex'

It does not work either even without the space in the path. IMHO this should be fixed.

C:\work\avr\avrdude_test\avrdude_bin> .\avrdude -c usbasp -p m328p -U "C:\work\avr\avrdude_test\avrdude_bin\hex\blink_uno_with_bootloader.hex"
avrdude error: invalid I/O mode '\' in update specification
  allowed values are:
    r = read device
    w = write device
    v = verify device
avrdude error: unable to parse update operation 'C:\work\avr\avrdude_test\avrdude_bin\hex\blink_uno_with_bootloader.hex'
stefanrueger commented 1 year ago

I needed to look at the parsing of -U and noted a trick how to solve the issue of Windows drive letters: As memory names don't contain colons and the r/w/v memory operation is a single character, it is sufficient to check whether the first two colons sandwich one character. That's enough to differentiate between -U <memory>:<op>:<file>[:<fmt>] and -U <file>[:<fmt>]. Changing the -U parsing in that way will get C:/some/file.hex (with or without the final :i) passed. I've just implemented that idea in commit fdedba8 of PR #1358 and it works like a treat.

mcuee commented 1 year ago

@stefanrueger

Indeed PR #1358 seems to fix this issue.

PS C:\work\avr\avrdude_test\avrdude_bin> .\avrdude_pr1358v2_msvc.exe -C .\avrdude_pr1358v2_msvc.conf
 -c usbasp -p m328p -U C:\work\avr\avrdude_test\avrdude_bin\urboot_m328p.hex
avrdude:[os_find_busses] found bus-0
avrdude:[os_find_devices] found \\?\usb#vid_16c0&pid_05dc#0001#{a5dcbf10-6530-11d2-901f-00c04fb951ed}--WinUSB on bus-0
avrdude_pr1358v2_msvc: AVR device initialized and ready to accept instructions
avrdude_pr1358v2_msvc: device signature = 0x1e950f (probably m328p)
avrdude_pr1358v2_msvc: Note: flash memory has been specified, an erase cycle will be performed.
                       To disable this feature, specify the -D option.
avrdude_pr1358v2_msvc: erasing chip
avrdude_pr1358v2_msvc: reading input file C:\work\avr\avrdude_test\avrdude_bin\urboot_m328p.hex for flash
                       with 350 bytes in 2 sections within [0x7e00, 0x7fff]
                       using 4 pages and 162 pad bytes
avrdude_pr1358v2_msvc: writing 350 bytes flash ...
Writing | ################################################## | 100% 0.04 s
avrdude_pr1358v2_msvc: 350 bytes of flash written
avrdude_pr1358v2_msvc: verifying flash memory against C:\work\avr\avrdude_test\avrdude_bin\urboot_m328p.hex
Reading | ################################################## | 100% 0.00 s
avrdude_pr1358v2_msvc: 350 bytes of flash verified

avrdude_pr1358v2_msvc done.  Thank you.
mcuee commented 1 year ago

@stefanrueger

Windws file path name is working under the terminal mode as well.

PS C:\work\avr\avrdude_test\avrdude_bin> .\avrdude_pr1358v2_msvc.exe -C .\avrdude_pr1358v2_msvc.conf -c usbasp -p m328p -e
avrdude:[os_find_busses] found bus-0
avrdude:[os_find_devices] found \\?\usb#vid_16c0&pid_05dc#0001#{a5dcbf10-6530-11d2-901f-00c04fb951ed}--WinUSB on bus-0
avrdude_pr1358v2_msvc: AVR device initialized and ready to accept instructions
avrdude_pr1358v2_msvc: device signature = 0x1e950f (probably m328p)
avrdude_pr1358v2_msvc: erasing chip

avrdude_pr1358v2_msvc done.  Thank you.

PS C:\work\avr\avrdude_test\avrdude_bin> .\avrdude_pr1358v2_msvc.exe -C .\avrdude_pr1358v2_msvc.conf -c usbasp -p m328p -t
avrdude:[os_find_busses] found bus-0
avrdude:[os_find_devices] found \\?\usb#vid_16c0&pid_05dc#0001#{a5dcbf10-6530-11d2-901f-00c04fb951ed}--WinUSB on bus-0
avrdude_pr1358v2_msvc: AVR device initialized and ready to accept instructions
avrdude_pr1358v2_msvc: device signature = 0x1e950f (probably m328p)
avrdude> write flash 0 C:\work\avr\avrdude_test\avrdude_bin\urboot_m328p.hex
Caching | ################################################## | 100% 0.09 s
avrdude> flush
avrdude_pr1358v2_msvc: synching cache to device ...
Writing | ################################################## | 100% 0.19 s
avrdude> quit

avrdude_pr1358v2_msvc done.  Thank you.

PS C:\work\avr\avrdude_test\avrdude_bin> .\avrdude_pr1358v2_msvc.exe -C .\avrdude_pr1358v2_msvc.conf -c usbasp -p m328p -U flash:v:C:\work\avr\avrdude_test\avrdude_bin\urboot_m328p.hex:i
avrdude:[os_find_busses] found bus-0
avrdude:[os_find_devices] found \\?\usb#vid_16c0&pid_05dc#0001#{a5dcbf10-6530-11d2-901f-00c04fb951ed}--WinUSB on bus-0
avrdude_pr1358v2_msvc: AVR device initialized and ready to accept instructions
avrdude_pr1358v2_msvc: device signature = 0x1e950f (probably m328p)
avrdude_pr1358v2_msvc: verifying flash memory against C:\work\avr\avrdude_test\avrdude_bin\urboot_m328p.hex
Reading | ################################################## | 100% 0.00 s
avrdude_pr1358v2_msvc: 350 bytes of flash verified

avrdude_pr1358v2_msvc done.  Thank you.