Skip to content

time_unix: namespace zephyr headers - #383

Merged
clalancette merged 1 commit into
ros2:rollingfrom
uLipe:feature/zephyr_namespace
Sep 30, 2022
Merged

clalancette merged 1 commit into
ros2:rollingfrom
uLipe:feature/zephyr_namespace

Conversation

@uLipe

@uLipe uLipe commented Sep 8, 2022

Copy link
Copy Markdown

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

uLipe commented Sep 8, 2022

Copy link
Copy Markdown
Author

CC: @pablogs9

@uLipe

uLipe commented Sep 14, 2022

Copy link
Copy Markdown
Author

@clalancette @ahcorde PTAL

Comment thread src/time_unix.c Outdated
@clalancette clalancette added the more-information-needed Further information is required label Sep 29, 2022
@uLipe
uLipe force-pushed the feature/zephyr_namespace branch from 7bf8043 to 9cbf466 Compare September 29, 2022 17:58
@uLipe

uLipe commented Sep 29, 2022

Copy link
Copy Markdown
Author

@pablogs9 @clalancette @ahcorde PTAL again.

Comment thread src/time_unix.c Outdated
@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
uLipe force-pushed the feature/zephyr_namespace branch from 0df2ddc to be19c37 Compare September 29, 2022 22:51
@uLipe

uLipe commented Sep 29, 2022

Copy link
Copy Markdown
Author

@fujitatomoya PTAL again also.

@pablogs9

Copy link
Copy Markdown
Member

The new way to check version LGTM

@clalancette clalancette left a comment

Copy link
Copy Markdown
Contributor

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
Copy Markdown
Contributor

CI:

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

@uLipe

uLipe commented Sep 30, 2022

Copy link
Copy Markdown
Author

@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
Copy Markdown
Collaborator

good to go with green CI.

@clalancette

clalancette commented Sep 30, 2022 •

Copy link
Copy Markdown
Contributor

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

pablogs9 commented Oct 3, 2022 •

Copy link
Copy Markdown
Member

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

@uLipe

uLipe commented Oct 3, 2022

Copy link
Copy Markdown
Author

@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