Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add warning message if gdb is not installed/found. #211

Open
gudnimg opened this issue Jun 13, 2021 · 8 comments
Open

Add warning message if gdb is not installed/found. #211

gudnimg opened this issue Jun 13, 2021 · 8 comments

Comments

@gudnimg
Copy link
Contributor

gudnimg commented Jun 13, 2021

With CMake 3.18 there is a REQUIRED option for find_program()

Example: If the arm-none-eabi-gcc is not found, the CMake will stop processing and create an error message.

CMake >= 3.18:

find_program(CMAKE_C_COMPILER NAMES ${STM32_TARGET_TRIPLET}-gcc PATHS ${TOOLCHAIN_BIN_PATH} REQUIRED)

CMake < 3.18:

find_program(CMAKE_C_COMPILER NAMES ${STM32_TARGET_TRIPLET}-gcc PATHS ${TOOLCHAIN_BIN_PATH})
@gudnimg
Copy link
Contributor Author

gudnimg commented Jun 17, 2021

We could also do this to preserve CMake 3.13 compatibility:

find_program(CMAKE_C_COMPILER NAMES ${STM32_TARGET_TRIPLET}-gcc PATHS ${TOOLCHAIN_BIN_PATH})

if (NOT CMAKE_C_COMPILER)
    message(FATAL_ERROR "${STM32_TARGET_TRIPLET}-gcc not found at ${TOOLCHAIN_BIN_PATH}.")
endif()

If some find_program() lines are not required. We could use WARNING instead of FATAL_ERROR in message().

Edit: The reason I created this issue was I for some reason did not have arm-none-eabi-gdb installed and CMake didn't raise an error to let me know. This can happen when arm-none-eabi is installed directly through the terminal manually.

@gudnimg gudnimg changed the title Bump minimum CMake version to 3.18 to improve find_program()? Bump minimum CMake version to 3.18 to improve find_program()? Jun 17, 2021
@gudnimg
Copy link
Contributor Author

gudnimg commented Jun 17, 2021

Here's an example of what the warning looks like. I think having a WARNING is better than nothing. I'll create an PR for this :) Screenshot from 2021-06-17 13-42-26

@atsju
Copy link
Collaborator

atsju commented Jun 20, 2021

This might also be worth an own issue.. There are also some calls to find utilities in the common.cmake file:

find_program(CMAKE_OBJCOPY NAMES ${STM32_TARGET_TRIPLET}-objcopy PATHS ${TOOLCHAIN_BIN_PATH} NO_DEFAULT_PATH)
find_program(CMAKE_OBJDUMP NAMES ${STM32_TARGET_TRIPLET}-objdump PATHS ${TOOLCHAIN_BIN_PATH} NO_DEFAULT_PATH)
find_program(CMAKE_SIZE NAMES ${STM32_TARGET_TRIPLET}-size PATHS ${TOOLCHAIN_BIN_PATH} NO_DEFAULT_PATH)
find_program(CMAKE_DEBUGGER NAMES ${STM32_TARGET_TRIPLET}-gdb PATHS ${TOOLCHAIN_BIN_PATH} NO_DEFAULT_PATH)
find_program(CMAKE_CPPFILT NAMES ${STM32_TARGET_TRIPLET}-c++filt PATHS ${TOOLCHAIN_BIN_PATH} NO_DEFAULT_PATH)

I am used to add the toolchain binary path to the system path (temporarily) and then simply look for arm-none-eabi-XYZ instead of setting a specific variable like STM32_TOOLCHAIN_PATH . CMake caches the path at build generation so the environment stays unpolluted after build generation has been done once. The later one allows multiple toolchains (although I guess this is not really relevant for most users).. I don't know the exact reasoning behind the NO_DEFAULT_PATH option, but maybe it might be a good idea to omit that option. Those utilities are not really required, but every toolchain should have them, so maybe REQUIRED can be added there as well.

Hello @rmspacefish
As you wrote yourself, please consider opening a dedicated issue for this. It's not directly linked to original issue and will cause confusion.

@atsju
Copy link
Collaborator

atsju commented Jun 20, 2021

Hello @gudnimg
I think keeping the 3.13 compatibility is better option as it is the default distributed version in many Linux distributions. Your proposal to get a warning instead error seems good but no arm-none-eabi-gcc shall create an error. Only -gdb is optional and should get a warning.

Please consider doing a pull request doing the modification and referencing the issue.

@gudnimg
Copy link
Contributor Author

gudnimg commented Jun 21, 2021

@atsju Thanks for the reply. Your comment about 3.13 makes a lot of sense now (I’ve seen other repos also use 3.13 as minimum)

I will submit a PR. It may be today or later this week. 😊

@atsju
Copy link
Collaborator

atsju commented Jun 26, 2021

@gudnimg I looked at your PR #218.
For my understanding, could you explain why you needed arm-none-eabi-gdb ? I mean this is not directly linked to cmake so it's great to have a warning if it's not detected but did you really use the (-NOTFOUND) variable CMAKE_DEBUGGER or you needed gdb externally of cmake ?

@gudnimg
Copy link
Contributor Author

gudnimg commented Jun 26, 2021

@atsju I use it externally. Though now that you mention it... does the repo need to find arm-none-eabi-gdb? It would be nice if one could use CMake to flash the STM32 chip with custom commands :)

Edit:

For example this is one way to do it with NRF52 (based on https://github.com/Polidea/cmake-nRF5x), for STM32 we could swap nrfjprog for gdb.

function(flash_application_firmware
    target # target name
)
    # Generate HEX file
    convert_to_hex(${target})

    add_custom_target(flash 
        COMMAND # Flash application firmware
            ${NRF5_NRFJPROG} --log --program "${target}.hex" --family nrf52 --sectorerase ${nrfjprog_jlink_sn_opt} ${nrfjprog_jlink_sn_arg}
        COMMAND # Apply reset in order to start the application
            ${NRF5_NRFJPROG} --reset --family nrf52 ${nrfjprog_jlink_sn_opt} ${nrfjprog_jlink_sn_arg}
        COMMENT "Flashing application firmware.."
    )
endfunction()

Then when I want to flash the stm32 I just call ninja flash.

At the moment I am using the terminal directly to flash.

@atsju
Copy link
Collaborator

atsju commented Jun 26, 2021

does the repo need to find arm-none-eabi-gdb?

This is the point. But you proved that someone will find an use so we can keep it. But maybe it should be INFO instead WARNING? And then you should MESSAGE for each non-mandatory find_package not only gdb?

I let you think about it and update the PR.

@atsju atsju changed the title Bump minimum CMake version to 3.18 to improve find_program()? Add warning message if gdb is not installed/found. Jul 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants