bessman / mcbootflash

Flash firmware to 16-bit Microchip devices running MCC bootloader
https://bessman.github.io/mcbootflash/
MIT License
16 stars 2 forks source link

C++ version #44

Closed mathieugalle closed 9 months ago

mathieugalle commented 9 months ago

Hi,

First of all, thank you for creating this library. But I needed a solution to flash my PIC24s from my Linux embedded system without relying on Python.

Inspired by your library, I developed (or more accurately, copy-pasted-chatgpted) a C++ version that replicates its functionality. This is still an experimental version, and I've made it available here. The structure and function names mirror those of your library, with some necessary adaptations due to language differences (such as the absence of generators in C++).

Currently, it doesn't include a flashing utility like yours, but focuses on converting hex files into "chunks" and achieving the same results as your implementation.

The code isn't perfect yet, but it might be useful to someone in its current state.

If anyone has suggestions for improvements or would like to contribute, feel free to reach out or submit a pull request. Any feedback is welcome!

bessman commented 9 months ago

Cool!

Have you looked at existing C++ HEX libraries, like Intel HEX Class or libintelhex? I haven't used either myself, so I can't speak for them, but HEX parsing seems like a common enough problem that I would expect there to already exist C++ libraries for it.

Since this isn't a bug report or a feature request, I'm going to close it. But thanks for letting me know about your port :smiley:

mathieugalle commented 9 months ago

Hi, thank you for bringing up Intel Hex C++ libraries! Indeed, I could have considered using them.

However, I came across your discussions with the author of bincopy in this GitHub issue. It seems to me that the C++ Intel binaries might not offer the exact support needed for the very specialized microchip 24-bit format, which includes extended linear segments and other subtleties like having to double some addresses.

Besides, I also wanted to take this opportunity to understand the format myself directly. And be sure to have the minimal amount of code needed for the task.

Given this context, I thought it might be more practical to attempt a direct port of bincopy instead.

By the way, I noticed that in the flash.hex file used for unit tests, we're not testing the extended segments (IHEX_EXTENDED_LINEAR_ADDRESS). I'm not sure if it's necessary to test them in mcbootflash or bincopy, but I'm pointing it out because my hex file had this type of line. example :

:045e0000ffffff00a1
:020000040000fa.

(It is working correctly)

bessman commented 9 months ago

By the way, I noticed that in the flash.hex file used for unit tests, we're not testing the extended segments (IHEX_EXTENDED_LINEAR_ADDRESS). I'm not sure if it's necessary to test them in mcbootflash or bincopy, but I'm pointing it out because my hex file had this type of line. example :

I'm pretty sure bincopy has a test for extended linear address records. Testing the same thing in mcbootflash would be redundant, and a violation of separation of concerns; It is bincopy's job to parse the hex file, and mcbootflash's job to send the contents over the wire.

Notice that most of mcbootflash's functionality has no concept of hex files; instead they consume generalized firmware chunks. Mcbootflash only depends on bincopy via the chunked utility function. The firmware chunks could conceivably come from some other source.

The hex files in the unit tests are just a convenient way to store the test data. They could have been pickles or something instead.

mathieugalle commented 9 months ago

You are right, it looks like this issue is about extended segment and a test for this was indeed added here

And you are also right about separation of concerns : each library should focus on its own job. Since I redo bincopy, I should have this test, but this does not apply to this library.