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

time_unix: namespace zephyr headers #383

Merged
merged 1 commit into from
Sep 30, 2022

Conversation

uLipe
Copy link
Contributor

@uLipe uLipe commented Sep 8, 2022

After version 3, the Zephyr RTOS requires to namespace all its headers with zephyr before including any other header file that belongs to it, this PR checks the version o major number of the kernel and when above 3, namespace the Zephyr specific posix timer implementation.

@uLipe
Copy link
Contributor Author

uLipe commented Sep 8, 2022

CC: @pablogs9

@uLipe
Copy link
Contributor Author

uLipe commented Sep 14, 2022

@clalancette @ahcorde PTAL

src/time_unix.c Outdated Show resolved Hide resolved
@clalancette clalancette added the more-information-needed Further information is required label Sep 29, 2022
@uLipe
Copy link
Contributor Author

uLipe commented Sep 29, 2022

@pablogs9 @clalancette @ahcorde PTAL again.

src/time_unix.c Outdated Show resolved Hide resolved
@uLipe uLipe force-pushed the feature/zephyr_namespace branch 2 times, most recently from e2ff899 to 0df2ddc Compare September 29, 2022 22:46
when kernel is above version 3.

Signed-off-by: Felipe Neves <felipe.neves@linaro.org>
@uLipe
Copy link
Contributor Author

uLipe commented Sep 29, 2022

@fujitatomoya PTAL again also.

@pablogs9
Copy link
Member

The new way to check version LGTM

Copy link
Contributor

@clalancette clalancette left a comment

Choose a reason for hiding this comment

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

This is pretty messy, but if this makes it work, it is OK (actually this whole file is messy, but that's a separate problem).

Will run CI on it next.

@clalancette
Copy link
Contributor

CI:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

@uLipe
Copy link
Contributor Author

uLipe commented Sep 30, 2022

@clalancette , I agree on you, the solution is not the most elegant but needed to make this supported across Zephyr versions 2.7 and 3.1

the header namespacing in Zephyr is introducing the same problem in others external modules .

@clalancette clalancette removed the more-information-needed Further information is required label Sep 30, 2022
@fujitatomoya
Copy link
Collaborator

good to go with green CI.

@clalancette
Copy link
Contributor

clalancette commented Sep 30, 2022

Basically by definition, this change can't have made the one test on Windows fail. That one is flaky every once in a while anyway. Going ahead and merging this.

@pablogs9
Copy link
Member

pablogs9 commented Oct 3, 2022

PTAL here: micro-ROS/micro_ros_zephyr_module#85 (comment)
Zephyr 2.7.2 failing to find <zephyr/posix/time.h>
CC: @uLipe

@uLipe
Copy link
Contributor Author

uLipe commented Oct 3, 2022

@pablogs9 , reason and fix here: #390

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 this pull request may close these issues.

4 participants