CMake: Improve debug configuration #1051

StarGate01 · 2022-03-25T12:04:21Z

This PR has been broken out of #1050.

Extended debug configuration
~~Use correct linker script in RECOVERY_MCUBOOT~~ merged via src/CMakeLists.txt: use iHEX format for mcuboot image creation #256
~~Use iHEX format for mcuboot image creation~~ merged via src/CMakeLists.txt: use iHEX format for mcuboot image creation #256

CMakeLists.txt

src/CMakeLists.txt

StarGate01 · 2022-03-30T22:38:21Z

src/CMakeLists.txt

+          POST_BUILD
+          COMMAND adafruit-nrfutil dfu genpkg --dev-type 0x0052 --application ${IMAGE_MCUBOOT_RECOVERYLOADER_FILE_NAME} ${DFU_MCUBOOT_RECOVERYLOADER_FILE_NAME}
+          COMMENT "post build (DFU) steps for ${EXECUTABLE_MCUBOOT_RECOVERYLOADER_FILE_NAME}"
+          )
 endif()


 # FLASH
 if (USE_JLINK)


The diff looks a bit odd, I only corrected the indentation here as well.

doc/buildAndProgram.md

StarGate01 · 2022-05-10T14:41:11Z

I rebased this PR on the latest development version, and removed any hardware target configuration options. These will be moved to #1050 .

Riksu9000

The bin to hex change is blocking this currently, since there's concern that it may break compatibility with some companion apps. #256 also has this same change, so we'll reconsider that first.

src/CMakeLists.txt

Riksu9000 · 2022-05-13T08:39:11Z

The bin to hex change is blocking this currently, since there's concern that it may break compatibility with some companion apps. #256 also has this same change, so we'll reconsider that first.

The objcopy command is invoked by the CMake script as well, such that both the bin as well as the hex version are created. The binary file is exactly the same as before. Or are there concerns of tools depending on the inner workings of the CMake compilation chain?

Maybe something about the DFU. I'm not entirely sure. Someone else will need to take a closer look at it.

StarGate01 · 2022-05-14T01:47:39Z

The image creating tool tools/mcuboot/imgtool.py accepts both bin as well as hex files to create a DFU package.

JF002 · 2022-05-14T10:16:58Z

I unfortunately do not have much time to think about those changes right now. I'm currently working on improving the memory usage and monitoring, and I do not have much brain bandwidth for other things :)

This PR is not easy to read as it serves multiples goals (change the debug configuration behavior, switch from bin to hex, change indentation,...).

As I am the one who wrote the original script, I decided to use .bin file because that's what I'm used to use... I'm in full control of where the data will be flashed in memory and that fine for me. I know other people prefer the hex file because they don't have to worry about the offset in memory. I don't think one is better than the other, they are just different.

Now, other than this practical aspect, do we have other valid reasons to change how the files (firmwares, DFUs, mcuboot images) are generated?
Also, we need to pay attention to not break any companion app. I'm not sure all of them rely on the NRF library to parse the DFU file. I think at least Amazfish reads the .bin file directly for the OTA functionality.

This PR also changes the behavior of the Debug configuration by settings new variables. Not every developer will need them. Also, they are already defined in sdk_config.h. I'm wondering if these duplicate declarations/definitions won't be confusing...

StarGate01 · 2022-05-14T16:02:40Z

@JF002 Thanks for your feedback. I see that there are valid concerns about third party tools. How about I revert the process of the DFU generation back to its original state, but add a objcopy statement (unrelated to the DFU creation) to generate a hex file from the bin file in addition?

My main motivation to generate hex files is that the toolchain I use (J-Link / nrfjprog) can only process hex files. OpenOCD is somehow unstable on my system, and lacks some debugging functionality.

StarGate01 · 2022-05-17T19:42:30Z

The hex format change as well as the linker fix included in this PR are the same as in #256, since my P8 fork is based on the fork by @ildar . This is also noted in the commit messages.

JF002

@StarGate01 I've finally taken the time to check a few things : the .bin and .hex files are correctly generated, and the DFU file still contains a .bin file. So I guess my concerns were unfounded. Sorry about that!

src/CMakeLists.txt

JF002 · 2022-06-01T19:31:42Z

@Riksu9000 @StarGate01 Which PR should we merge? This one or #256?

StarGate01 · 2022-06-01T19:33:09Z

This PR should include all changes from #256, since I forked from @ildar orgininally.

Riksu9000 · 2022-06-02T06:18:37Z

@JF002 I would merge #256 first, but it doesn't make a big difference in the end.

JF002 · 2022-06-06T17:47:43Z

I've just merged ~~#265~~ #256 ;-)
But I guess the remaining changes in this PR still make sense? If yes, I'll merge it too !

NeroBurner · 2022-06-06T18:45:55Z

@JF002 I think you mean #256 instead of #265

JF002 · 2022-06-06T19:07:52Z

@JF002 I think you mean #256 instead of #265

Yep, my fingers slipped! I've just fixed my comment!

Enable debug output for InfiniTime, Nimble and the NRF SDK via SEGGER RTT on debug builds.

StarGate01 · 2022-06-06T19:39:07Z

I rebased this PR, it now contains only the debug config enhancements as well as the fixes for the indentation. All good to merge afaik, the two obsolete commits were the same as in #256 .

NeroBurner

I'm not using on device debugging, so I can't comment on that.

But the changes are simple and seem to be sound

Avamander

Looks better than before, I think.

JF002 · 2022-06-19T19:23:45Z

Thanks for this PR, @StarGate01 ! Looks good to me :-)

By the way, as you've touched the flash commands in the CMake file, may I ask your opinion about #801 ? It removes those custom targets as I think they are not generic/portable enough. I haven't dared to merge it as I'm afraid to confuse some developers who might use them.

This was referenced Mar 25, 2022

Nimble: Reduce power usage, add compatibility to LFCLK calibration #1052

Merged

Add initial P8 a/b smartwatch support #1050

Draft

NeroBurner requested changes Mar 30, 2022

View reviewed changes

CMakeLists.txt Outdated Show resolved Hide resolved

src/CMakeLists.txt Outdated Show resolved Hide resolved

src/CMakeLists.txt Outdated Show resolved Hide resolved

StarGate01 force-pushed the cmake-fixes branch from 077fd48 to d39b916 Compare March 30, 2022 21:55

StarGate01 commented Mar 30, 2022

View reviewed changes

StarGate01 requested a review from NeroBurner March 30, 2022 22:43

StarGate01 force-pushed the cmake-fixes branch from d39b916 to 82a1687 Compare April 4, 2022 16:18

Riksu9000 reviewed May 2, 2022

View reviewed changes

doc/buildAndProgram.md Outdated Show resolved Hide resolved

StarGate01 force-pushed the cmake-fixes branch from 82a1687 to f438458 Compare May 10, 2022 14:32

StarGate01 force-pushed the cmake-fixes branch from f438458 to 1f4f3f0 Compare May 10, 2022 20:17

Riksu9000 reviewed May 12, 2022

View reviewed changes

src/CMakeLists.txt Outdated Show resolved Hide resolved

Riksu9000 mentioned this pull request May 12, 2022

Add basic hardware configuration options for P8 #1128

Merged

StarGate01 force-pushed the cmake-fixes branch from 1f4f3f0 to c8f03c0 Compare May 13, 2022 00:41

StarGate01 force-pushed the cmake-fixes branch from c8f03c0 to 4243c54 Compare May 17, 2022 19:13

JF002 reviewed May 23, 2022

View reviewed changes

src/CMakeLists.txt Show resolved Hide resolved

StarGate01 force-pushed the cmake-fixes branch from 4243c54 to 0ef0082 Compare June 5, 2022 12:03

JF002 mentioned this pull request Jun 6, 2022

src/CMakeLists.txt: use iHEX format for mcuboot image creation #256

Merged

CMake: Extend debug configuration

e9a9022

Enable debug output for InfiniTime, Nimble and the NRF SDK via SEGGER RTT on debug builds.

CMake: Fix indentation

62fc95a

StarGate01 force-pushed the cmake-fixes branch from 0ef0082 to 62fc95a Compare June 6, 2022 19:20

StarGate01 changed the title ~~CMake: Fix linker scripts and formats, improve debug configuration~~ CMake: Improve debug configuration Jun 6, 2022

NeroBurner approved these changes Jun 6, 2022

View reviewed changes

Riksu9000 requested a review from JF002 June 12, 2022 07:38

Avamander approved these changes Jun 19, 2022

View reviewed changes

JF002 approved these changes Jun 19, 2022

View reviewed changes

JF002 added this to the 1.10.0 milestone Jun 19, 2022

JF002 merged commit 91c69d3 into InfiniTimeOrg:develop Jun 19, 2022

StarGate01 mentioned this pull request Jun 19, 2022

Remove CMake targets specific to the debugging environment #801

Merged

StarGate01 deleted the cmake-fixes branch June 25, 2022 17:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CMake: Improve debug configuration #1051

CMake: Improve debug configuration #1051

StarGate01 commented Mar 25, 2022 •

edited

Loading

StarGate01 Mar 30, 2022

StarGate01 commented May 10, 2022

Riksu9000 left a comment

Riksu9000 commented May 13, 2022

StarGate01 commented May 14, 2022

JF002 commented May 14, 2022

StarGate01 commented May 14, 2022 •

edited

Loading

StarGate01 commented May 17, 2022 •

edited

Loading

JF002 left a comment

JF002 commented Jun 1, 2022

StarGate01 commented Jun 1, 2022

Riksu9000 commented Jun 2, 2022

JF002 commented Jun 6, 2022 •

edited

Loading

NeroBurner commented Jun 6, 2022

JF002 commented Jun 6, 2022

StarGate01 commented Jun 6, 2022 •

edited

Loading

NeroBurner left a comment

Avamander left a comment

JF002 commented Jun 19, 2022

CMake: Improve debug configuration #1051

CMake: Improve debug configuration #1051

Conversation

StarGate01 commented Mar 25, 2022 • edited Loading

StarGate01 Mar 30, 2022

Choose a reason for hiding this comment

StarGate01 commented May 10, 2022

Riksu9000 left a comment

Choose a reason for hiding this comment

Riksu9000 commented May 13, 2022

StarGate01 commented May 14, 2022

JF002 commented May 14, 2022

StarGate01 commented May 14, 2022 • edited Loading

StarGate01 commented May 17, 2022 • edited Loading

JF002 left a comment

Choose a reason for hiding this comment

JF002 commented Jun 1, 2022

StarGate01 commented Jun 1, 2022

Riksu9000 commented Jun 2, 2022

JF002 commented Jun 6, 2022 • edited Loading

NeroBurner commented Jun 6, 2022

JF002 commented Jun 6, 2022

StarGate01 commented Jun 6, 2022 • edited Loading

NeroBurner left a comment

Choose a reason for hiding this comment

Avamander left a comment

Choose a reason for hiding this comment

JF002 commented Jun 19, 2022

StarGate01 commented Mar 25, 2022 •

edited

Loading

StarGate01 commented May 14, 2022 •

edited

Loading

StarGate01 commented May 17, 2022 •

edited

Loading

JF002 commented Jun 6, 2022 •

edited

Loading

StarGate01 commented Jun 6, 2022 •

edited

Loading