search

platformio / platform-atmelavr

Atmel AVR: development platform for PlatformIO
https://registry.platformio.org/platforms/platformio/atmelavr
Apache License 2.0
140 stars 105 forks source link

excessive quotes fix #319

Closed thijses closed 1 year ago

thijses commented 1 year ago

spaces in the file path require quotation marks, but the current version (4.2.0) adds too many quotation marks. These small fixes restore the number of quotes. (did something recently change in the way scons env.Replace() handles lists of strings?) (i have not tested this in linux, but i'm hoping python/scons is consistent across platforms)

valeros commented 1 year ago

Thanks for the PR, merged!

thijses commented 1 year ago

Hi @valeros Thank you for your assistance in the merger, but there has been a slight issue. In my initial commit, i also changed line 137 (now 136): https://github.com/platformio/platform-atmelavr/blob/90aa1f1db25c30c10cae8721db3fddb90cf38444/builder/bootloader.py#L136 to:

BOOTFLAGS='-Uflash:w:"%s":i' % bootloader_path  +" "+  "-Ulock:w:%s:m" % lock_bits,
# can't be list, because of doubled quotes around bootloader_path

While i agree that list for is much cleaner (as you changed it back in the tidy up commit), this does actually re-introduce an issue if your windows path has spaces in it. It appeas to me that env.Replace() is adding "" quotes around any list-item with spaces in it, regardless of whether the item already has "" quotes in it. This results in the UPLOADBOOTCMD: avrdude -p atmega328p -C "C:\Users\Thijs van Liempd\.platformio\packages\tool-avrdude\avrdude.conf" -c usbasp "-Uflash:w:"C:\Users\Thijs van Liempd\.platformio\packages\framework-arduino-avr\bootloaders\optiboot/optiboot_atmega328.hex":i" -Ulock:w:0x0F:m which has BOOTFLAGS: "-Uflash:w:"C:\Users\Thijs van Liempd\.platformio\packages\framework-arduino-avr\bootloaders\optiboot/optiboot_atmega328.hex":i" -Ulock:w:0x0F:m with doubled quotes around the first argument.

Is there a way to prevent env.Replace (or perhaps the list-to-string conversion is done at a later stage) from adding these extra (otherwise well-intentioned) quotes? If not, then i'm afraid my unsightly string-addition fix should probably be reinstated, if you want to actually allow bootloader flashing for (silly) people with spaces in their path.

valeros commented 1 year ago

this does actually re-introduce an issue if your windows path has spaces in it

My bad, I was focused on the issue with the path to AVRDUDE configuration file. I've pushed a possible fix for this issue, could you please pull the latest changes and try again?

thijses commented 1 year ago

sorry, but i'm still a little new to proper Git usage. Where have you pushed this fix? (can you specify exactly what you'd like me to try?)

valeros commented 1 year ago

Where have you pushed this fix? (can you specify exactly what you'd like me to try?)

Just specify this repository in the platform field, for example:

[env:ATmega3290]
; Note: a Git client is required!
platform = https://github.com/platformio/platform-atmelavr.git
framework = arduino
board = ATmega3290
thijses commented 1 year ago

aaah, i see (i was overcomplicating things). Of course i tried the development branch; that is where the strings in my comment from yesterday came from. Nevertheless, here is a complete verbose 'burn bootloader' command output with a fresh devlopment branch platform:

PS C:\Users\Thijs van Liempd\Documents\firmware\Roof\roof_TLD> pio run --target bootloader --environment nanoatmega328new -v
Processing nanoatmega328new (platform: https://github.com/platformio/platform-atmelavr.git; board: nanoatmega328new; framework: arduino; upload_protocol: usbasp; monitor_speed: 115200) 
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------CONFIGURATION: https://docs.platformio.org/page/boards/atmelavr/nanoatmega328new.html
PLATFORM: Atmel AVR (4.2.0+sha.90aa1f1) (git+https://github.com/platformio/platform-atmelavr.git) > Arduino Nano ATmega328 (New Bootloader)
HARDWARE: ATMEGA328P 16MHz, 2KB RAM, 30KB Flash
DEBUG: Current (avr-stub) External (avr-stub, simavr)
PACKAGES:
 - framework-arduino-avr @ 5.1.0
 - tool-avrdude @ 1.60300.200527 (6.3.0)
 - toolchain-atmelavr @ 1.70300.191015 (7.3.0)
LDF: Library Dependency Finder -> https://bit.ly/configure-pio-ldf
LDF Modes: Finder ~ chain, Compatibility ~ soft
Found 5 compatible libraries
Scanning dependencies...
No dependencies
Building in release mode
Using bootloader image:
C:\Users\Thijs van Liempd\.platformio\packages\framework-arduino-avr\bootloaders\optiboot/optiboot_atmega328.hex

Selected fuses: [lfuse = 0xFF, hfuse = 0xDA, efuse = 0xFD]
avrdude -p atmega328p -C "C:\Users\Thijs van Liempd\.platformio\packages\tool-avrdude\avrdude.conf" -e -c usbasp -Ulock:w:0x3F:m -Uhfuse:w:0xDA:m -Ulfuse:w:0xFF:m -Uefuse:w:0xFD:m

avrdude: AVR device initialized and ready to accept instructions

Reading | ################################################## | 100% 0.00s

avrdude: Device signature = 0x1e950f (probably m328p)
avrdude: erasing chip
avrdude: reading input file "0x3F"
avrdude: writing lock (1 bytes):

Writing | ################################################## | 100% 0.00s

avrdude: 1 bytes of lock written
avrdude: verifying lock memory against 0x3F:
avrdude: load data lock data from input file 0x3F:
avrdude: input file 0x3F contains 1 bytes
avrdude: reading on-chip lock data:

Reading | ################################################## | 100% 0.00s

avrdude: verifying ...
avrdude: 1 bytes of lock verified
avrdude: reading input file "0xDA"
avrdude: writing hfuse (1 bytes):

Writing | ################################################## | 100% 0.00s

avrdude: 1 bytes of hfuse written
avrdude: verifying hfuse memory against 0xDA:
avrdude: load data hfuse data from input file 0xDA:
avrdude: input file 0xDA contains 1 bytes
avrdude: reading on-chip hfuse data:

Reading | ################################################## | 100% 0.00s

avrdude: verifying ...
avrdude: 1 bytes of hfuse verified
avrdude: reading input file "0xFF"
avrdude: writing lfuse (1 bytes):

Writing | ################################################## | 100% 0.00s

avrdude: 1 bytes of lfuse written
avrdude: verifying lfuse memory against 0xFF:
avrdude: load data lfuse data from input file 0xFF:
avrdude: input file 0xFF contains 1 bytes
avrdude: reading on-chip lfuse data:

Reading | ################################################## | 100% 0.00s
avrdude -p atmega328p -C "C:\Users\Thijs van Liempd\.platformio\packages\tool-avrdude\avrdude.conf" -c usbasp "-Uflash:w:"C:\Users\Thijs van Liempd\.platformio\packages\framework-arduino-avr\bootloaders\optiboot/optiboot_atmega328.hex":i" -Ulock:w:0x0F:m

avrdude: verifying ...
avrdude: 1 bytes of lfuse verified
avrdude: reading input file "0xFD"
avrdude: writing efuse (1 bytes):

Writing | ################################################## | 100% 0.00s

avrdude: 1 bytes of efuse written
avrdude: verifying efuse memory against 0xFD:
avrdude: load data efuse data from input file 0xFD:
avrdude: input file 0xFD contains 1 bytes
avrdude: reading on-chip efuse data:

Reading | ################################################## | 100% 0.00s

avrdude: verifying ...
avrdude: 1 bytes of efuse verified

avrdude: safemode: Fuses OK (E:FD, H:DA, L:FF)

avrdude done.  Thank you.

avrdude: invalid file format '\Users\Thijs' in update specifier
avrdude: error parsing update operation 'flash:w:C:\Users\Thijs'
*** [bootloader] Error 1
============================================================================== [FAILED] Took 1.21 seconds ==============================================================================
valeros commented 1 year ago

Could you please also run pio pkg update -g -p https://github.com/platformio/platform-atmelavr.git and try again?

thijses commented 1 year ago

i think my other packages were already up-to-date:

PS C:\Users\Thijs van Liempd\Documents\firmware\Roof\roof_TLD> pio pkg update -g -p https://github.com/platformio/platform-atmelavr.git
Platform Manager: Updating atmelavr @ 4.2.0+sha.90aa1f1
git version 2.37.0.windows.1
remote: Enumerating objects: 7, done.
remote: Counting objects: 100% (7/7), done.
remote: Compressing objects: 100% (1/1), done.
remote: Total 4 (delta 3), reused 4 (delta 3), pack-reused 0
Unpacking objects: 100% (4/4), 369 bytes | 11.00 KiB/s, done.
From https://github.com/platformio/platform-atmelavr
   90aa1f1..b270c70  develop    -> origin/develop
Updating 90aa1f1..b270c70
Fast-forward
 builder/bootloader.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Tool Manager: framework-arduino-avr@5.1.0 is already up-to-date
Tool Manager: tool-avrdude@1.60300.200527 is already up-to-date
Tool Manager: toolchain-atmelavr@1.70300.191015 is already up-to-date

only the commit from 45 minutes ago updated. Now it does indeed work:

PS C:\Users\Thijs van Liempd\Documents\firmware\Roof\roof_TLD> pio run --target bootloader --environment nanoatmega328new -v
Processing nanoatmega328new (platform: https://github.com/platformio/platform-atmelavr.git; board: nanoatmega328new; framework: arduino; upload_protocol: usbasp; monitor_speed: 115200)
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------CONFIGURATION: https://docs.platformio.org/page/boards/atmelavr/nanoatmega328new.html
PLATFORM: Atmel AVR (4.2.0+sha.b270c70) (git+https://github.com/platformio/platform-atmelavr.git) > Arduino Nano ATmega328 (New Bootloader)
HARDWARE: ATMEGA328P 16MHz, 2KB RAM, 30KB Flash
DEBUG: Current (avr-stub) External (avr-stub, simavr)
PACKAGES:
 - framework-arduino-avr @ 5.1.0
 - tool-avrdude @ 1.60300.200527 (6.3.0)
 - toolchain-atmelavr @ 1.70300.191015 (7.3.0)
LDF: Library Dependency Finder -> https://bit.ly/configure-pio-ldf
LDF Modes: Finder ~ chain, Compatibility ~ soft
Found 5 compatible libraries
Scanning dependencies...
No dependencies
Building in release mode
Using bootloader image:
C:\Users\Thijs van Liempd\.platformio\packages\framework-arduino-avr\bootloaders\optiboot/optiboot_atmega328.hex

Selected fuses: [lfuse = 0xFF, hfuse = 0xDA, efuse = 0xFD]
avrdude -p atmega328p -C "C:\Users\Thijs van Liempd\.platformio\packages\tool-avrdude\avrdude.conf" -e -c usbasp -Ulock:w:0x3F:m -Uhfuse:w:0xDA:m -Ulfuse:w:0xFF:m -Uefuse:w:0xFD:m

avrdude: AVR device initialized and ready to accept instructions

Reading | ################################################## | 100% 0.00s

avrdude: Device signature = 0x1e950f (probably m328p)
avrdude: erasing chip
avrdude: reading input file "0x3F"
avrdude: writing lock (1 bytes):

Writing | ################################################## | 100% 0.00s

avrdude: 1 bytes of lock written
avrdude: verifying lock memory against 0x3F:
avrdude: load data lock data from input file 0x3F:
avrdude: input file 0x3F contains 1 bytes
avrdude: reading on-chip lock data:

Reading | ################################################## | 100% 0.00s

avrdude: verifying ...
avrdude: 1 bytes of lock verified
avrdude: reading input file "0xDA"
avrdude: writing hfuse (1 bytes):

Writing | ################################################## | 100% 0.00s

avrdude: 1 bytes of hfuse written
avrdude: verifying hfuse memory against 0xDA:
avrdude: load data hfuse data from input file 0xDA:
avrdude: input file 0xDA contains 1 bytes
avrdude: reading on-chip hfuse data:

Reading | ################################################## | 100% 0.00s

avrdude: verifying ...
avrdude: 1 bytes of hfuse verified
avrdude: reading input file "0xFF"
avrdude: writing lfuse (1 bytes):

Writing | ################################################## | 100% 0.00s

avrdude: 1 bytes of lfuse written
avrdude: verifying lfuse memory against 0xFF:
avrdude: load data lfuse data from input file 0xFF:
avrdude: input file 0xFF contains 1 bytes
avrdude: reading on-chip lfuse data:

Reading | ################################################## | 100% 0.00s

avrdude: verifying ...
avrdude: 1 bytes of lfuse verified
avrdude: reading input file "0xFD"
avrdude: writing efuse (1 bytes):

Writing | #################avrdude -p atmega328p -C "C:\Users\Thijs van Liempd\.platformio\packages\tool-avrdude\avrdude.conf" -c usbasp "-Uflash:w:C:\Users\Thijs van Liempd\.platformio\packages\framework-arduino-avr\bootloaders\optiboot/optiboot_atmega328.hex:i" -Ulock:w:0x0F:m
################################# | 100% 0.00s

avrdude: 1 bytes of efuse written
avrdude: verifying efuse memory against 0xFD:
avrdude: load data efuse data from input file 0xFD:
avrdude: input file 0xFD contains 1 bytes
avrdude: reading on-chip efuse data:

Reading | ################################################## | 100% 0.00s

avrdude: verifying ...
avrdude: 1 bytes of efuse verified

avrdude: safemode: Fuses OK (E:FD, H:DA, L:FF)

avrdude done.  Thank you.

avrdude: AVR device initialized and ready to accept instructions

Reading | ################################################## | 100% 0.00s

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:\Users\Thijs van Liempd\.platformio\packages\framework-arduino-avr\bootloaders\optiboot/optiboot_atmega328.hex"
avrdude: writing flash (32768 bytes):

Writing | ################################################## | 100% 0.00s

avrdude: 32768 bytes of flash written
avrdude: verifying flash memory against C:\Users\Thijs van Liempd\.platformio\packages\framework-arduino-avr\bootloaders\optiboot/optiboot_atmega328.hex:
avrdude: load data flash data from input file C:\Users\Thijs van Liempd\.platformio\packages\framework-arduino-avr\bootloaders\optiboot/optiboot_atmega328.hex:
avrdude: input file C:\Users\Thijs van Liempd\.platformio\packages\framework-arduino-avr\bootloaders\optiboot/optiboot_atmega328.hex contains 32768 bytes
avrdude: reading on-chip flash data:

Reading | ################################################## | 100% -0.00s

avrdude: verifying ...
avrdude: 32768 bytes of flash verified
avrdude: reading input file "0x0F"
avrdude: writing lock (1 bytes):

Writing | ################################################## | 100% 0.01s

avrdude: 1 bytes of lock written
avrdude: verifying lock memory against 0x0F:
avrdude: load data lock data from input file 0x0F:
avrdude: input file 0x0F contains 1 bytes
avrdude: reading on-chip lock data:

Reading | ################################################## | 100% 0.00s

avrdude: verifying ...
avrdude: 1 bytes of lock verified

avrdude: safemode: Fuses OK (E:FD, H:DA, L:FF)

avrdude done.  Thank you.

============================================================================== [SUCCESS] Took 1.91 seconds ==============================================================================

TL;DR: "-Uflash:w:C:\Users\Thijs van Liempd\.platformio\packages\framework-arduino-avr\bootloaders\optiboot/optiboot_atmega328.hex:i" is still wrapped in qoutes in its entirety, but avrdude doesn't seem to mind. I'm tempted to question why this didn't work before, or maybe my brain broke and i just didn't even try removing the quotes that way. In any case, it works now, looks great!, thank you :)