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

move RTPS to dedicated px4fmu-v{3,4,4pro,5}, posix, sdflight builds #8113

Merged
merged 1 commit into from
Oct 17, 2017

Conversation

dagar
Copy link
Member

@dagar dagar commented Oct 11, 2017

RTPS and micro-cdr now only included and required in the following builds

- nuttx_px4fmu-v3_rtps
- nuttx_px4fmu-v4_rtps
- nuttx_px4fmu-v4pro_rtps
- nuttx_px4fmu-v5_rtps

- posix_sitl_rtps

- posix_eagle_rtps
- posix_excelsior_rtps

@dagar dagar requested review from mcharleb, davids5 and bkueng October 11, 2017 18:36
davids5
davids5 previously approved these changes Oct 11, 2017
Copy link
Member

@davids5 davids5 left a comment

Choose a reason for hiding this comment

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

@dagar - This looks good. Is the CI failure real?

@dagar
Copy link
Member Author

dagar commented Oct 11, 2017

No, that's been failing intermittently.

bkueng
bkueng previously approved these changes Oct 12, 2017
Copy link
Member

@bkueng bkueng left a comment

Choose a reason for hiding this comment

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

This looks good

lib/micro-CDR
)

set(config_rtps_send_topics
Copy link
Member

Choose a reason for hiding this comment

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

Can we move the set of topics to common/rtps? Will be easier to manage.

Copy link
Member Author

Choose a reason for hiding this comment

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

That sounds fine to me, but I don't think we know how this is actually intended to be used.

@@ -209,6 +213,14 @@ alt_firmware: \
check_s2740vc-v1_default \
sizes

# builds with RTPS
check_rtps: \
Copy link
Member

Choose a reason for hiding this comment

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

Is the plan to add this to CI? If so, we'll need to check the overhead of building all targets.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, this is for CI.

@dagar
Copy link
Member Author

dagar commented Oct 17, 2017

Apologies for continuing to change this, but what do you think about everything living within modules/micrortps_bridge? micro-CDR, generation, topic lists, etc

Little things like this will help as we work towards large restructuring for modularity (#8127, #8139).

See updated push.

@dagar dagar dismissed stale reviews from bkueng and davids5 via 6557cd8 October 17, 2017 14:34
@mcharleb
Copy link
Contributor

Looks good. I'd still like to be able to use the generated agent code without having to sync and build PX4 to get it.

@dagar
Copy link
Member Author

dagar commented Oct 17, 2017

We could have Firmware output a library (with a cmake build system) that's automatically pushed to another github repo. That generated library could be versioned to match Firmware.

@dagar
Copy link
Member Author

dagar commented Oct 17, 2017

Merging now to get it added to semaphoreci. I suspect this will go through a few more iterations once we figure out what to do with the generated agent code and the directory structure refactor.

@dagar dagar merged commit d83073f into PX4:master Oct 17, 2017
@dagar dagar deleted the pr-rtps_builds branch October 17, 2017 20:29
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