Skip to content
This repository has been archived by the owner on Apr 17, 2023. It is now read-only.

Fix Fail to Build Examples if ARDUINO_SDK_PATH is not in Default Paths #89

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

escrowdis
Copy link

If the variable ARDUINO_SDK_PATH is given by the command line

cmake ... -DARDUINO_SDK_PATH=<path-of-arduino-sdk>

rather than

export ARDUINO_SDK_PATH=<path-of-arduino-sdk>

the examples are failed to build and output error in "ArduinoSDKSeeker.cmake" with unknown reason, though it's cached.

- If the variable ARDUINO_SDK_PATH is given by the command line
  `cmake ... -DARDUINO_SDK_PATH=<path-of-arduino-sdk>`
  rather than `export ARDUINO_SDK_PATH=<path-of-arduino-sdk>`,
  the examples are failed to build and output error in
  ArduinoSDKSeeker.cmake with unknown reason, though it's cached.
Copy link
Member

@MrPointer MrPointer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job detecting this one, it has annoyed a lot of people recently, and I had no actual explanation/solution other than defining an environment variable for this.

@@ -67,6 +67,7 @@ else ()
# Set default path if none is set
find_arduino_sdk(arduino_sdk_path)
set(ARDUINO_SDK_PATH "${arduino_sdk_path}" CACHE PATH "Arduino SDK Path")
set(ENV{ARDUINO_SDK_PATH} "${ARDUINO_SDK_PATH}")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm I would wrap it in a conditional that applies only in the case where the env var isn't defined, as it's user's responsibility to set it.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree. This behavior is really weird even the variable has been cached in the file CMakeCache.txt. I've checked this set won't affect the environment variables. I'll track on this to make a better solution.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants