search

platformio / platform-atmelmegaavr

Atmel megaAVR: development platform for PlatformIO
https://registry.platformio.org/platforms/platformio/atmelmegaavr
Apache License 2.0
29 stars 21 forks source link

Add support for 1200bps touch #37

Closed MCUdude closed 2 years ago

MCUdude commented 2 years ago

Add support for 1200bps touch improve upload port handling. Now -P USB is added if the programmer is not a jtag2updi, and -P $UPLOAD_PORT if the programmer is a jtag2updi. This can be done because the jtag2updi programmer is currently the only UPDI programmer that communicates over UART.

Would be great if you can test and verify that it works for you @maxgerhardt

Closes #34

maxgerhardt commented 2 years ago

This fails for me.

>pio run -t fuses -v
Processing nano_every (platform: https://github.com/MCUdude/platform-atmelmegaavr.git#improve-fuses-script; board: nano_every; framework: arduino)
--------------------------------------------------------------------------------------------------------------------------------------------
CONFIGURATION: https://docs.platformio.org/page/boards/atmelmegaavr/nano_every.html
PLATFORM: Atmel megaAVR (1.5.0+sha.60e74f9) (git+https://github.com/MCUdude/platform-atmelmegaavr.git#improve-fuses-script) > Arduino Nano Every
HARDWARE: ATMEGA4809 16MHz, 6KB RAM, 47.50KB Flash
PACKAGES:
 - framework-arduino-megaavr 1.8.7
 - tool-avrdude-megaavr 1.60300.191015 (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
Auto-detected: COM11

Selected fuses:
-------------------------
[fuse2 / osccfg   = 0x01]
[fuse5 / syscfg0  = 0xC9]
[lock  / lockbit  = 0xC5]
-------------------------

avrdude -p atmega4809 -C C:\Users\Max\.platformio\packages\tool-avrdude-megaavr@1.60300.191015\avrdude.conf -v -P "COM11" -c jtag2updi -Ufuse2:w:0x01:m -Ufuse5:w:0xC9:m -Ulock:w:0xC5:m

avrdude: Version 6.3-20190619
         Copyright (c) 2000-2005 Brian Dean, http://www.bdmicro.com/
         Copyright (c) 2007-2014 Joerg Wunsch

         System wide configuration file is "C:\Users\Max\.platformio\packages\tool-avrdude-megaavr@1.60300.191015\avrdude.conf"

         Using Port                    : COM11
         Using Programmer              : jtag2updi
avrdude: jtagmkII_getsync(): sign-on command: status -1
avrdude: jtagmkII_getsync(): sign-on command: status -1
avrdude: jtagmkII_getsync(): sign-on command: status -1
avrdude: jtagmkII_getsync(): sign-on command: status -1
avrdude: jtagmkII_getsync(): sign-on command: status -1
avrdude: jtagmkII_getsync(): sign-on command: status -1
avrdude: jtagmkII_getsync(): sign-on command: status -1
avrdude: jtagmkII_getsync(): sign-on command: status -1
avrdude: jtagmkII_getsync(): sign-on command: status -1
^Cscons: *** [fuses] Build interrupted.
Error: Aborted by user

Testing with

[env:nano_every]
platform = https://github.com/MCUdude/platform-atmelmegaavr.git#improve-fuses-script
board = nano_every
framework = arduino

Uploading still works as normal.

BeforeUpload(["upload"], [".pio\build\nano_every\firmware.hex"])
Use manually specified: COM11
Forcing reset using 1200bps open/close on port COM11
avrdude -v -p atmega4809 -C C:\Users\Max\.platformio\packages\tool-avrdude-megaavr@1.60300.191015\avrdude.conf -c jtag2updi -Ufuse2:w:0x01:m -Ufuse5:w:0xC9:m -Ulock:w:0xC5:m -b 115200 -V -e -D -P "COM11" -U flash:w:.pio\build\nano_every\firmware.hex:i

avrdude: Version 6.3-20190619
         Copyright (c) 2000-2005 Brian Dean, http://www.bdmicro.com/
         Copyright (c) 2007-2014 Joerg Wunsch

         System wide configuration file is "C:\Users\Max\.platformio\packages\tool-avrdude-megaavr@1.60300.191015\avrdude.conf"

         Using Port                    : COM11
         Using Programmer              : jtag2updi
         Overriding Baud Rate          : 115200
JTAG ICE mkII sign-on message:
Communications protocol version: 1
M_MCU:
  boot-loader FW version:        1
  firmware version:              6.00
  hardware version:              1
S_MCU:
  boot-loader FW version:        1
  firmware version:              6.00
  hardware version:              1
Serial number:                   00:00:00:00:00:00
Device ID:                       JTAGICE mkII
         AVR Part                      : ATmega4809
         Chip Erase delay              : 0 us
         PAGEL                         : P00
         BS2                           : P00
         RESET disposition             : dedicated
         RETRY pulse                   : SCK
         serial program mode           : yes
         parallel program mode         : yes
         Timeout                       : 0
         StabDelay                     : 0
         CmdexeDelay                   : 0
         SyncLoops                     : 0
         ByteDelay                     : 0
         PollIndex                     : 0
         PollValue                     : 0x00
         Memory Detail                 :

                                  Block Poll               Page                       Polled
           Memory Type Mode Delay Size  Indx Paged  Size   Size #Pages MinW  MaxW   ReadBack
           ----------- ---- ----- ----- ---- ------ ------ ---- ------ ----- ----- ---------
           signature      0     0     0    0 no          3    0      0     0     0 0x00 0x00
           prodsig        0     0     0    0 no         61   61      0     0     0 0x00 0x00
           fuses          0     0     0    0 no          9    0      0     0     0 0x00 0x00
           fuse0          0     0     0    0 no          1    0      0     0     0 0x00 0x00
           fuse1          0     0     0    0 no          1    0      0     0     0 0x00 0x00
           fuse2          0     0     0    0 no          1    0      0     0     0 0x00 0x00
           fuse4          0     0     0    0 no          1    0      0     0     0 0x00 0x00
           fuse5          0     0     0    0 no          1    0      0     0     0 0x00 0x00
           fuse6          0     0     0    0 no          1    0      0     0     0 0x00 0x00
           fuse7          0     0     0    0 no          1    0      0     0     0 0x00 0x00
           fuse8          0     0     0    0 no          1    0      0     0     0 0x00 0x00
           lock           0     0     0    0 no          1    0      0     0     0 0x00 0x00
           data           0     0     0    0 no          0    0      0     0     0 0x00 0x00
           usersig        0     0     0    0 no         64   64      0     0     0 0x00 0x00
           flash          0     0     0    0 no      49152  128      0     0     0 0x00 0x00
           eeprom         0     0     0    0 no        256   64      0     0     0 0x00 0x00

         Programmer Type : JTAGMKII_PDI
         Description     : JTAGv2 to UPDI bridge
         M_MCU hardware version: 1
         M_MCU firmware version: 6.00
         S_MCU hardware version: 1
         S_MCU firmware version: 6.00
         Serial number:          00:00:00:00:00:00
         Vtarget         : 5.0 V

avrdude: jtagmkII_initialize(): Cannot locate "flash" and "boot" memories in description
avrdude: AVR device initialized and ready to accept instructions

Reading | ################################################## | 100% 0.27s

avrdude: Device signature = 0x1e9651 (probably m4809)
avrdude: erasing chip
avrdude: reading input file "0x01"
avrdude: writing fuse2 (1 bytes):

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

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

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

avrdude: verifying ...
avrdude: 1 bytes of fuse2 verified
avrdude: reading input file "0xC9"
avrdude: writing fuse5 (1 bytes):

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

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

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

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

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

avrdude: 1 bytes of lock written
avrdude: verifying lock memory against 0xC5:
avrdude: load data lock data from input file 0xC5:
avrdude: input file 0xC5 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 ".pio\build\nano_every\firmware.hex"
avrdude: writing flash (802 bytes):

Writing | ################################################## | 100% 0.65s

avrdude: 802 bytes of flash written

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

avrdude done.  Thank you.

======================================================= [SUCCESS] Took 5.84 seconds =======================================================
MCUdude commented 2 years ago

What happends if you add board_upload.use_1200bps_touch = true to platformio.ini?

maxgerhardt commented 2 years ago

It does set the fuses, but funny enough it stops for about 2 seconds waiting for a "new upload port" after doing the 1200bps reset.

Building in release mode
Auto-detected: COM11

Selected fuses:
-------------------------
[fuse2 / osccfg   = 0x01]
[fuse5 / syscfg0  = 0xC9]
[lock  / lockbit  = 0xC5]
-------------------------

Use manually specified: COM11
Forcing reset using 1200bps open/close on port COM11
Waiting for the new upload port...
avrdude -p atmega4809 -C C:\Users\Max\.platformio\packages\tool-avrdude-megaavr@1.60300.191015\avrdude.conf -v -P "COM11" -c jtag2updi -Ufuse2:w:0x01:m -Ufuse5:w:0xC9:m -Ulock:w:0xC5:m

avrdude: Version 6.3-20190619
         Copyright (c) 2000-2005 Brian Dean, http://www.bdmicro.com/
         Copyright (c) 2007-2014 Joerg Wunsch

         System wide configuration file is "C:\Users\Max\.platformio\packages\tool-avrdude-megaavr@1.60300.191015\avrdude.conf"

         Using Port                    : COM11
         Using Programmer              : jtag2updi
JTAG ICE mkII sign-on message:
Communications protocol version: 1
M_MCU:
  boot-loader FW version:        1
  firmware version:              6.00
  hardware version:              1
S_MCU:
  boot-loader FW version:        1
  firmware version:              6.00
  hardware version:              1
Serial number:                   00:00:00:00:00:00
Device ID:                       JTAGICE mkII
         AVR Part                      : ATmega4809
         Chip Erase delay              : 0 us
         PAGEL                         : P00
         BS2                           : P00
         RESET disposition             : dedicated
         RETRY pulse                   : SCK
         serial program mode           : yes
         parallel program mode         : yes
         Timeout                       : 0
         StabDelay                     : 0
         CmdexeDelay                   : 0
         SyncLoops                     : 0
         ByteDelay                     : 0
         PollIndex                     : 0
         PollValue                     : 0x00
         Memory Detail                 :

                                  Block Poll               Page                       Polled
           Memory Type Mode Delay Size  Indx Paged  Size   Size #Pages MinW  MaxW   ReadBack
           ----------- ---- ----- ----- ---- ------ ------ ---- ------ ----- ----- ---------
           signature      0     0     0    0 no          3    0      0     0     0 0x00 0x00
           prodsig        0     0     0    0 no         61   61      0     0     0 0x00 0x00
           fuses          0     0     0    0 no          9    0      0     0     0 0x00 0x00
           fuse0          0     0     0    0 no          1    0      0     0     0 0x00 0x00
           fuse1          0     0     0    0 no          1    0      0     0     0 0x00 0x00
           fuse2          0     0     0    0 no          1    0      0     0     0 0x00 0x00
           fuse4          0     0     0    0 no          1    0      0     0     0 0x00 0x00
           fuse5          0     0     0    0 no          1    0      0     0     0 0x00 0x00
           fuse6          0     0     0    0 no          1    0      0     0     0 0x00 0x00
           fuse7          0     0     0    0 no          1    0      0     0     0 0x00 0x00
           fuse8          0     0     0    0 no          1    0      0     0     0 0x00 0x00
           lock           0     0     0    0 no          1    0      0     0     0 0x00 0x00
           data           0     0     0    0 no          0    0      0     0     0 0x00 0x00
           usersig        0     0     0    0 no         64   64      0     0     0 0x00 0x00
           flash          0     0     0    0 no      49152  128      0     0     0 0x00 0x00
           eeprom         0     0     0    0 no        256   64      0     0     0 0x00 0x00

         Programmer Type : JTAGMKII_PDI
         Description     : JTAGv2 to UPDI bridge
         M_MCU hardware version: 1
         M_MCU firmware version: 6.00
         S_MCU hardware version: 1
         S_MCU firmware version: 6.00
         Serial number:          00:00:00:00:00:00
         Vtarget         : 5.0 V

avrdude: jtagmkII_initialize(): Cannot locate "flash" and "boot" memories in description
avrdude: AVR device initialized and ready to accept instructions

Reading | ################################################## | 100% 0.27s

avrdude: Device signature = 0x1e9651 (probably m4809)
avrdude: reading input file "0x01"
avrdude: writing fuse2 (1 bytes):

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

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

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

avrdude: verifying ...
avrdude: 1 bytes of fuse2 verified
avrdude: reading input file "0xC9"
avrdude: writing fuse5 (1 bytes):

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

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

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

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

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

avrdude: 1 bytes of lock written
avrdude: verifying lock memory against 0xC5:
avrdude: load data lock data from input file 0xC5:
avrdude: input file 0xC5 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:FF, H:FF, L:FF)

avrdude done.  Thank you.

======================================================= [SUCCESS] Took 10.66 seconds =======================================================
MCUdude commented 2 years ago

It does set the fuses, but funny enough it stops for about 2 seconds waiting for a "new upload port" after doing the 1200bps reset.

This is intended. PlatformIO connects with a baudrate of 1200bps. When connection is established the jtag2updi programmer resets, port disappear, and starts up again, but this time in programming mode. Arduino Leonardo works the same way.

Can you try to remove board_upload.use_1200bps_touch = true form platformio.ini and add "use_1200bps_touch": true, to nano_every.json, like shown below? If this works the 1200bps touch will always happens automatically if using a Nano Every and the official Arduino core, not MegaCoreX.

  "upload": {
    "extra_flags": ["-V", "-e", "-D"],
    "maximum_ram_size": 6144,
    "maximum_size": 48640,
    "protocol": "jtag2updi",
    "use_1200bps_touch": true,
    "speed": 115200
  },
maxgerhardt commented 2 years ago

Can you try to remove board_upload.use_1200bps_touch = true form platformio.ini and add "use_1200bps_touch": true, to nano_every.json, like shown below?

Interestingly enough, then it does not work anymore, no reset is done. I'd also think that the changes were equivalent. It only works when I edit the JSON file as

    "use_1200bps_touch": "true"

aka, as a string, not a bool. Possibly there's some type mismatch in the Python script?

MCUdude commented 2 years ago

aka, as a string, not a bool. Possibly there's some type mismatch in the Python script?

I'm by no means a Python expert, but from what I understand, everything that's defined in platformio.ini are treated as trings, hence why "yes" were in quotes. However, true/false in the json manifest file are treated as bools.

Try replacing line 261 in fuses.py with this line. This would make use_1200bps_touch work in the json manifest file and in platformio.ini.

if upload_options.get("use_1200bps_touch", False) in (True, "true"):
maxgerhardt commented 2 years ago

Reference boards from official platforms like

https://github.com/platformio/platform-atmelavr/blob/develop/boards/pro8MHzatmega328.json#L31

do not use a string for boolean values in the board JSON, so this should be the way to go.

if upload_options.get("use_1200bps_touch", False) in (True, "true"):

This works in conjunction with "use_1200bps_touch": true in the board manifest.

MCUdude commented 2 years ago

Thanks for the feedback! I've pushed some changes. First, use_1200bps_touch is now added to nano_every.json by default, and the fuses.py scripts can handle both bools and strings

valeros commented 2 years ago

Hi @MCUdude , thanks for the PR. I believe the platform already has similar functionality in the main.py script. Why don't we reuse the "touch" logic from the main script? This way we won't duplicate similar code in two places. It may look something line this:

#
# Target: Setup fuses
#

fuses_action = None
if "fuses" in COMMAND_LINE_TARGETS:
    fuses_actions = [
        env.VerboseAction(BeforeUpload, "Looking for port..."),
        env.SConscript("fuses.py", exports="env")
    ]

env.AddPlatformTarget("fuses", None, fuses_actions, "Set Fuses")
MCUdude commented 2 years ago

@valeros this will work in this case, because Arduino Nano Every has pre-defined fuses, and the Uno Wifi Rev2 uses fuses_file instead.

But for everyone else, it's not straightforward why this would work. just because fuses is present in the manifest file shouldn't automatically enable 1200bps touch. What if newer boards get added in the future that has pre-defined fuses but uses a different kind of programmer? That would break this logic.

valeros commented 2 years ago

I'm not sure I'm following. The presence of the fuses section in any manifest has nothing to do with the changes I proposed above. If you take a look at the content of the BeforeUpload function https://github.com/platformio/platform-atmelmegaavr/blob/develop/builder/main.py#L24-L50 it won't run the 1200bps touch unless it's set in the manifest.

MCUdude commented 2 years ago

If you take a look at the content of the BeforeUpload function https://github.com/platformio/platform-atmelmegaavr/blob/develop/builder/main.py#L24-L50 it won't run the 1200bps touch unless it's set in the manifest.

OK, then I understand. But it's important that 1200bps touch works if not defined in the manifest file, but instead in platformio.ini. Will this work? For instance, if the Nano Every is used with MegaCoreX (ATmega4809.json), and the user wants to set the fuses

valeros commented 2 years ago

From the standpoint of the main build script there is no difference if this setting is set in the board manifest or via the board_upload.use_1200bps_touch option directly in the platformio.ini file.

MCUdude commented 2 years ago

So I tried replacing the existing code in main.py with the one below and removed the 1200bps touch handler I added to fuses.py. However, now it's always trying to do a 1200bps touch, even if there's no use_1200bps_touch: true line in the manifest file. For some reason, upload_options.get("use_1200bps_touch", False) in BeforeUpload always returns true.

#
# Target: Setup fuses
#

fuses_action = None
if "fuses" in COMMAND_LINE_TARGETS:
    fuses_actions = [
        env.VerboseAction(BeforeUpload, "Looking for port..."),
        env.SConscript("fuses.py", exports="env")
    ]

env.AddPlatformTarget("fuses", None, fuses_actions, "Set Fuses")
valeros commented 2 years ago

It seems the use_1200bps_touch option is dynamically set to True for the jtag2updi protocol: https://github.com/platformio/platform-atmelmegaavr/blob/develop/builder/main.py#L204

I guess it isn't necessary always true for the fuses programming? It can be easily fixed by checking the current target:

if upload_protocol == "jtag2updi" and "fuses" not in COMMAND_LINE_TARGETS:
    upload_options["require_upload_port"] = True
    upload_options["use_1200bps_touch"] = True
    upload_options["wait_for_upload_port"] = False
elif upload_protocol == "arduino":
    upload_options["require_upload_port"] = True
    upload_options["use_1200bps_touch"] = False
    upload_options["wait_for_upload_port"] = False
    env.Append(UPLOADERFLAGS=["-D"])
MCUdude commented 2 years ago

Yes, I realized this right after I wrote the previous message. I think I found a solution that will work. BTW what is the default return value for upload_options["wait_for_upload_port"] if not present in the manifest file or platformio.ini? (BTW I've taken the liberty to future-proof this script by adding a new programmer, "Serialupdi". This will be present in the next version of Avrdude (7.0), and lets you upload to any UPDI target using standard USB to serial adapter, identical to how pyupdi/pymcuprog does it).

    if upload_protocol in ("jtag2updi", "serialupdi"):
        upload_options["require_upload_port"] = True
    elif upload_protocol == "arduino":
        upload_options["require_upload_port"] = True
        upload_options["use_1200bps_touch"] = False
        upload_options["wait_for_upload_port"] = False
        env.Append(UPLOADERFLAGS=["-D"])
valeros commented 2 years ago

BTW what is the default return value for upload_options["wait_for_upload_port"] if not present in the manifest file or platformio.ini?

It returns None, Didn't notice you asked for a different way of accessing values. Basically it's just a generic Python dictionary, so it will raise an exception. It's a good practice to use .get and set a safe default value just in case.

if upload_protocol in ("jtag2updi", "serialupdi"):
        upload_options["require_upload_port"] = True

Shouldn't two other options use_1200bps_touch and wait_for_upload_port be properly configured as well for these two protocols?

MCUdude commented 2 years ago

use_1200bps_touch is a Nano Every specific thing and is not needed on generic jtag2updi programmers, Only on the Nano Every. Usually, wait_for_upload_port is required when use_1200bps_touch has been used, for some reason, this has not been done here, but it works fine without any wait time.

A 1200bps touch is needed on the Nano Every since the programming chip has a dual purpose, a jtag2updi programmer and a USB to serial adapter. I can always add wait_for_upload_port: true to the Nano every manifest file just to be on the safe side. It should have been true from the beginning.

The same applies to SerialUPDI. SerialUPDI does not require a 1200bps touch or want_for_upload_port.

EDIT: FYI I do have a Nano Every that I test with to confirm that everything works as expected on real hardware

valeros commented 2 years ago

I believe the use_1200bps_touch option originated in your PR #5 that suggested using it even for generic ATmega targets with jtag2updi. I guess it safe to disable these dynamic configuration for jtag2updi and specify it explicitly only for Nano Every?

MCUdude commented 2 years ago

I guess it safe to disable these dynamic configuration for jtag2updi and specify it explicitly only for Nano Every?

Yes, I've also tested with a standalone jtag2updi programmer (just an Arduino UNO running the jtag2updi firmware), and it works perfectly without a 1200bps touch. Not sure why I added this to all jtag2updi programmers in #5...

The PR has been updated to reflect these changes.

EDIT: I've just rebased and force-pushed an update that hopefully addresses the issue the CI picked up.

MCUdude commented 2 years ago

The last question is, should the Nano Every really have "wait_for_upload_port": true, in its manifest file or not? It has worked excellent before without, and uploading is actually faster without since there isn't really necessary to wait for the new port to arrive.

@valeros do you have any preferences? Do it like it has been done in the past that has been proven to work, or to it "by the book"?

valeros commented 2 years ago

I'm no expert here, if it works just fine without that field, then feel free to remove it.

MCUdude commented 2 years ago

I'm no expert here, if it works just fine without that field, then feel free to remove it.

Thanks! I've removed the commit that added the line in the manifest file

valeros commented 2 years ago

LGTM, Let me know if you don't plan to add any further changes, I'll merge.

MCUdude commented 2 years ago

Let me know if you don't plan to add any further changes, I'll merge.

I think we're good now. I'll open a separate issue or PR if there are other bugs or features that need to be addressed. Thank you for your support!

valeros commented 2 years ago

Thanks, merged.