-
Notifications
You must be signed in to change notification settings - Fork 47
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
Support android #165
Support android #165
Conversation
repositories/cyclonedds.BUILD.bazel
Outdated
@@ -99,7 +107,7 @@ cache_entries_linux_and_macos = { | |||
cache_entries_qnx = { | |||
"ENABLE_SOURCE_SPECIFIC_MULTICAST": "OFF", | |||
"ENABLE_IPV6": "OFF", | |||
"CMAKE_SYSTEM_NAME": "QNX", | |||
"CMAKE_SYSTEM_NAME": "QNX_OR_ANDROID", |
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.
Is it good to call it QNX_OR_ANDROID
& cache_entries_qnx
?
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.
Should we add CI for different platforms also? @mvukov
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.
The naming is fine. Thanks for the contribution. BTW, can you share how would you use ROS+android?
Regarding CI: ATM I dunno how would we set up CI for Android. IMO, this PR would be good to go as-is, once the CI is green.
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.
Sure. We focus on the AR/VR usage for car cabin and both automotive OS and AR/VR devices may use Android.
A simplest CI could be just the compilation for @platforms//os:android
?
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.
Well, compilation doesn't mean that e.g. a deployment works. Do Python-based ROS 2 deployments (ros2_launch targets) work on Android for you? IMO, this is a bit more involved issue -- we can leave this for a follow-up.
299d2bf
to
21be637
Compare
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.
Very nice work! I left a couple of small comments.
repositories/cyclonedds.BUILD.bazel
Outdated
@@ -99,7 +107,7 @@ cache_entries_linux_and_macos = { | |||
cache_entries_qnx = { | |||
"ENABLE_SOURCE_SPECIFIC_MULTICAST": "OFF", | |||
"ENABLE_IPV6": "OFF", | |||
"CMAKE_SYSTEM_NAME": "QNX", | |||
"CMAKE_SYSTEM_NAME": "QNX_OR_ANDROID", |
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.
Well, compilation doesn't mean that e.g. a deployment works. Do Python-based ROS 2 deployments (ros2_launch targets) work on Android for you? IMO, this is a bit more involved issue -- we can leave this for a follow-up.
21be637
to
d0429fe
Compare
Thanks for the contribution! 💯 |
Use the same configs with QNX and it's working.