microchip-pic-avr-tools / pymcuprog

a Python utility for programming various Microchip MCU devices using Microchip CMSIS-DAP based debuggers
MIT License
96 stars 22 forks source link

No CLI Documentation #7

Closed WestfW closed 2 years ago

WestfW commented 2 years ago

There doesn't seem to be any documentation for using pymcuprog via the CLI, other than that provided by the "pymcuprog --help" command. How about creating such a file, even if it no more than a copied output of the help command?

Having to browse the source or install and run the program just to see what commands are available is sub-optimal!

xedbg commented 2 years ago

Good idea @WestfW There is more 'help' available on pypi.md (https://github.com/microchip-pic-avr-tools/pymcuprog/blob/main/pypi.md) but its not as direct. (Logged internally as DSG-3943)

T3sl4co1l commented 2 years ago

Also no information on file formats. Based on my observations, *.hex (it's Intel format by the way) can only be used for flash. Specifying any other type, it will be read as literal (binary) instead.

Nothing says what size e.g. fuses.bin must be; testing with an avr64da32 I find it requires 16 bytes, even though the device only specifies 9 (two of which are reserved anyway).

Hmm, playing around, it seems a full 16 bytes are actually there? Undocumented memory? Very peculiar as I have no way to specify that in the source; the FUSES macro is one particular type and that's it.

xedbg commented 2 years ago

Hmmm, I guess we take it for granted - we mostly work with Intel .hex files and literal values using -l. Binary files are somewhat 'dangerous' in my opinion, but cheap to implement support for. We should document this better, yes.

Regarding fuses: this is taken from the data model for the device: https://github.com/microchip-pic-avr-tools/pymcuprog/blob/main/pymcuprog/deviceinfo/devices/avr64da32.py#L24 So it appears to have 16 fuses... This is harvested from the public .atdf (packs.download.microchip.com): <memory-segment exec="0" name="FUSES" pagesize="0x1" rw="RW" size="0x10" start="0x00001050" type="fuses"/> If its wrong, then we will fix it to 8 (never mind what's behind the curtain :)

But writing 4 bytes to a 8 (or 16) byte area should NOT require the entire page be supplied if the write size (granularity) is 1 - this sounds like a bug. (Indeed its possible to write individual fuse bytes as literal values)

T3sl4co1l commented 2 years ago

Yeah, I see that; it also checks out experimentally -- what's missing seems to be the datasheet itself (DS40002233B), only showing offsets 0-8, but it turns out the offsets 9 to 0xf also exist, and so should be labeled "reserved". I don't know if you have an easier time reporting this to MCP (I'm not seeing any juicy "report error" buttons offhand..).

The fixed size makes things more difficult as, the FUSES macro uses a struct, and manually putting more stuff in .fuses "causes a section type conflict with '__fuse'". It seems this is a notoriously unhelpful (albeit rare?) message so I'll have to see if there's some easy workaround (otherwise, it seems making a new section and merging them in the link script would do, just annoying to have to customize it).

xedbg commented 2 years ago

Update: Our bad - in harvesting the .atdf data into our python stack we took the size of the segment rather than the size of the register-group. The segment has 16 bytes allocated to it, but only 0 to 8 are implemented (so, no, there is nothing else there.) Solution for pymcuprog will be to reduce size from 16 to 9. Logged (DSG-3952) But: I am quite capable of writing a single byte from a binary file to the fuse space: pymcuprog write -m fuses -o 0 -f test_fuses.bin you claim it "requires 16 bytes" ?

T3sl4co1l commented 2 years ago

Sorry, requires up to 16, errors on more. Wait, then why did I have this problem in the first---oh, I think that might be patched then, somewhere in this process I updated from 3.9.1 to 3.9.3 so some of my earlier observations may be out of date, dangit.

It also accepts fewer than 9, which seems kinda eh, but yeah you can write 'em one at a time if you want, maybe the user intends to write just those (and which, if hex were supported, could be only those at specified offsets, even).

BTW, when can it ever be invoked without -d, or is it written that way for convenience (another doc error?)?

xedbg commented 2 years ago

Ah. FYI we release "beta" on test.pypi.org (currently 3.9.7) and push to production pypi less frequently. If you use an Xplained pro/mini/nano or Curiosity Nano (or similar) kit, the device name is read from the kit, so -d is not needed unless you mod the board to program/debug another device.

xedbg commented 2 years ago

added help.md in 3.10.2 https://github.com/microchip-pic-avr-tools/pymcuprog/blob/main/help.md this will probably grow and be tweaked over time. Thanks for the input!

T3sl4co1l commented 2 years ago

Thanks, that's much clearer!

xedbg commented 2 years ago

Also good points, thanks - logged a new one: DSG-3972 (Your initial input was well-timed during release week, and sufficiently cosmetic to sneak in. But documentation is continuously improved)

Any further suggestions, please post them here!

WestfW commented 2 years ago

I think the fuse issue is somewhat the fault of avr-gcc/avr-libc. If you include the fuses in the source code (via the FUSES macro), they want ALL the fuses and create a single linker section that contains values for all of the fuses. That made sense with the older AVRs where all the options got scatter/gathered into two or three bytes of fuses, but it's less convenient with the UDPI AVRs where there are more fuses, many of which make sense to set individually.) (for example, I'd really like optiboot_x source to be able to specify "set the BOOTEND fuse to 2, but don't bother touching any of the other fuses. And AFAIK, I can't.)

T3sl4co1l commented 2 years ago

Yeah, not really any good way to specify that; you'll have to RMW them manually, I suppose. (What's a good way to script that, anyway? Can the -- hey wait, does the program return ERRORLEVEL from a single byte read? Okay, probably dumb, because plenty of reasons, but just saying, that'd be cute...) Otherwise, would need a specific AND/OR/XOR mode.

Whereas, the compile-time values, make sense from a production standpoint: you want this configuration, and that's that.