-
Notifications
You must be signed in to change notification settings - Fork 3k
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
BLE: fix conflicting include by qualifying cordio pal includes #10042
Conversation
This requires the matching client change |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed all pal_
headers
@@ -31,7 +31,7 @@ | |||
#include "hci_api.h" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
out of curiosity - hci_ is not using relative paths like util or pal, why?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because it's not part of pal. This is an external codebase. This is not a valid long term solution and is a stopgap until we fix our build system.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This problem has been solved many times and we don't have to develop any cutting edge technology, mbed_lib.json can have a list of include dirs it exports and other libs that require them can import them:
so for cordio host stack it would be "export_include": ["stack/include/wsf/"]
and each library that needed it like cordio controller "depends": ["CORDIO"] which will import the includes that were exported.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM - thanks @paul-szczepanek-arm
I've marked this "needs preceding PR" but presumably the other changes are to client code so won't affect us releasing ? |
Indeed @adbridge 👍 |
CI started |
Test run: SUCCESSSummary: 13 of 13 test jobs passed |
Description
Fix #10022 by adding the full path to the include.
Pull request type
Reviewers
@donatieng
Release Notes