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

_library_search_process excludes valid libraries when there isn't a matching header. #19

Closed
raphael-bmec-co opened this issue Apr 24, 2020 · 6 comments
Labels
bug Something isn't working

Comments

@raphael-bmec-co
Copy link

In Arduino-CMake-Toolchain-release-1.1-dev\Arduino\System\BoardBuildTargets.cmake at the end of the function function(_library_search_process lib search_paths_var search_suffixes_var return_var) is this final check:

	# Although we got the match, let us search for the required header within the folder
	file(GLOB_RECURSE lib_header_path "${matched_lib_path}/${lib}.h*")
	if (NOT lib_header_path)
		set ("${return_var}" "${lib}-NOTFOUND" PARENT_SCOPE)
		return()
	endif()

I am not sure if this applied to other libraries. But the ESP32 BLE Arduino library has some caveats:

  1. The library folder name is BLE. So the call has to be target_link_arduino_libraries(arc003-2 AUTO_PUBLIC PRIVATE BLE) even though that isn't the library name.
  2. The library name is 'ESP32 BLE Arduino'. The mismatch here does not seem to be an issue (probably because BLE is still in the name).
  3. There is no BLE.h or BLE.cpp in the src as the library is comprised of other files. This is the core issue that prevents this library from being found.

Possible fixes are to:

  1. Modify the final check in _library_search_process to maybe include a flag to skip that check or remove it.
  2. Add BLE.h or BLE.cpp to the library (not ideal).

For me, I have commented out the check for the time being.

Looking forward to your thoughts.

@a9183756-gh
Copy link
Owner

Thanks for your analysis and raising a separate issue for this.

I can see that the library search cannot not find "ESP32 BLE Arduino", and whereas Arduino IDE is able to find it.

In this case,

  • Included header: BLEDevice.h
  • Library name expected by the toolchain in target_arduino_link_libraries (as per the specification): BLEDevice
  • Library folder name: ESP32_BLE_Arduino

As the library folder name does not match with the include name (as required by the specification), the search fails. Arduino-IDE implements the search differently through pre-processing, and hence is able to succeed.

Temporary workaround: (1) Rename ESP32_BLE_Arduino folder to BLEDevice (Search for ESP32_BLE_Arduino in your home or documents folder). (2) Use "BLEDevice" in target_arduino_link_libraries.

Resolution for this current issue: Although this library does not seem to follow the specification, it seems to leave some info in library.properties, that could be used to fix the issue.

Permanent resolution: Implement the search logic similar to that of Arduino IDE.

Final check in _library_search_process was added for another issue. I will analyze how to skip this check if needed.

@a9183756-gh a9183756-gh added the bug Something isn't working label Apr 24, 2020
@raphael-bmec-co
Copy link
Author

Thank you.

I have uncommented the final check and implemented the workaround you have proposed.

@simoleone
Copy link

simoleone commented May 1, 2020

I think I found the same issue with Bluefruit52Lib when including the file bluefruit.h. Similarly, the library.properties file specifies includes=bluefruit.h.

Edit: It seems like the spec for libraries also has some info about how the include files are determined from library.properties. A comma-separated list in includes, or if not present, any .h file at the top-level in src/.

@a9183756-gh
Copy link
Owner

Yes, I saw that libraries spec as well. Although it doesn't specify the usage of includes field in the context of library search, it makes sense for the toolchain to use this field to solve this issue (Although, I believe, Arduino IDE may work even without this field). The current implementation in the toolchain tried to avoid reading all the library.properties files, and the specification seemed to favour this by specifying the algorithm in terms of just folder names. Currently implemented algorithm thus takes in include name and easily arrives at the best matching library folder. However, the algorithm required to fix the issue is to take in include name and arrive at the best library folder that provides the include, which means

  1. Scan all libraries.properties, and maintain a list of libraries and the corresponding includes, which will be additionally used for finding the library match.
  2. Allow target_link_arduino_libraries to take library name as well, in addition to include name.
  3. If user installs/removes any libraries (e.g. based on compilation error), re-index the libraries,
  4. Avoid linking with unnecessary libraries during automatic linking (Due to the difference in the way Arduino IDE and the toolchain implements automatic linking).

Let us attempt at least (1) in the current development version, for this issue #19. We will attempt a better approach that matches with Arduino IDE in the next release.

a9183756-gh added a commit that referenced this issue May 4, 2020
Library and include names in some of the libraries differ, which
prevented the toolchain in locating the library (issue #19). The
improved algorithm introduces library indexer to index the available
libraries, their names and includes, the library search algorithm
utilizes this info during the search to fix the issue #19.
@a9183756-gh
Copy link
Owner

a9183756-gh commented May 4, 2020

EDIT: With another fix (Make library target names consistent), this fix should work now correctly. Please verify.

Fix for this issue is available with the change #22. Although all the required features are in place, the algorithm is still not exactly the same as Arduino IDE, and hence some difference is expected. Notably, if a library provides <some-include>.h, and if the library folder name does not match with <some-include> and if <some-include>.h is not listed in the includes field of the library.properties, then automatic detection of such a library is explicitly disabled to avoid the error mentioned in point (4) in the above comment. It is still possible to link with such a library explicitly using the library name (instead of the include name <some-include>) in target_link_arduino_libraries.

a9183756-gh added a commit that referenced this issue May 9, 2020
Library and include names in some of the libraries differ, which
prevented the toolchain in locating the library (issue #19). The
improved algorithm introduces library indexer to index the available
libraries, their names and includes, the library search algorithm
utilizes this info during the search to fix the issue #19.
a9183756-gh added a commit that referenced this issue May 9, 2020
* Improved library search algorithm

Library and include names in some of the libraries differ, which
prevented the toolchain in locating the library (issue #19). The
improved algorithm introduces library indexer to index the available
libraries, their names and includes, the library search algorithm
utilizes this info during the search to fix the issue #19.

Also the internal target names of Arduino libraries are made consistent
as there is now a possibility to use both the include as well as library
names in target_link_arduino_libraries. This is to avoid multiple
targets of the same library.
@a9183756-gh
Copy link
Owner

Fixed in release-1.1-dev. Closing the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants