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

Add CMake support for MotoPlus builder #381

Open
wants to merge 3 commits into
base: kinetic-devel
Choose a base branch
from

Conversation

Gaspard-Bourgeois
Copy link

Well done Julien. This file is working great in our lab, I support the pull request.

@gavanderhoorn
Copy link
Member

gavanderhoorn commented Jan 18, 2021

Thanks for the PR.

I'm wondering though: this seems to duplicate quite a bit of information which is already part of the various .mps files and the existing visual studio solution file.

I'm not against merging this, but duplication would be something to avoid.

Could you not have reused the .mps files?

Don't generate from scratch mps files from CMakeLists.txt and use directly existing mps files to match existing file tree structure
@Julien-Livet
Copy link

Julien-Livet commented Jan 27, 2021

Thanks for the feed back !

Originally, I wrote this file to use another IDE than MotoPlus IDE. I talked about that with Yaskawa France and it was possible with MpBuilder, and possibly without having Visual Studio IDE but using Visual Studio file project. So the purpose of my original commit.

But for this project, it's more interesting to use each mps file and existing vxproj file, so the updated commit.

@gavanderhoorn
Copy link
Member

So to get it clear: this adds a CMake macro which generates command lines to instruct mpbuilder.exe (from the MotoPlus SDK) to build MotoROS. Correct?

Originally, I wrote this file to use another IDE than MotoPlus IDE. I talked about that with Yaskawa France and it was possible with MpBuilder, and possibly without having Visual Studio IDE but using Visual Studio file project. So the purpose of my original commit.

But for this project, it's more interesting to use each mps file and existing vxproj file, so the updated commit.

thanks for this comment, I know understand better what you're trying to do.

I believe it would be fine to have some infrastructure to build MotoROS with mpbuilder.exe but without Visual Studio. If that was your original intent, we could merge that here, but we'd probably have to a) make that very clear and b) not name it CMakeLists.txt, but give it some sort of suffix to clearly indicate the use-case.

@Julien-Livet
Copy link

So to get it clear: this adds a CMake macro which generates command lines to instruct mpbuilder.exe (from the MotoPlus SDK) to build MotoROS. Correct?

Exactly! Thanks for your answer!

I believe it would be fine to have some infrastructure to build MotoROS with mpbuilder.exe but without Visual Studio. If that was your original intent, we could merge that here, but we'd probably have to a) make that very clear and b) not name it CMakeLists.txt, but give it some sort of suffix to clearly indicate the use-case.

I agree with your comment, I will see to re-think the content on a clear way and propose an appropriated name for it. Thank you very much!

Copy link
Member

@gavanderhoorn gavanderhoorn left a comment

Choose a reason for hiding this comment

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

Marking as Changes Requested so we don't accidentally merge.

1rst file to support existing setup with Visual Studio
2nd file without existing Visual Studio files (generation on the fly)
Copy link

@Julien-Livet Julien-Livet left a comment

Choose a reason for hiding this comment

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

Update CMake support

@Gaspard-Bourgeois
Copy link
Author

it seems OK to me

@gavanderhoorn
Copy link
Member

I'm not sure I follow @Julien-Livet and @Gaspard-Bourgeois: why did you close the PR?

@dhled
Copy link

dhled commented Feb 19, 2021

Hey @gavanderhoorn . I think @Gaspard-Bourgeois didn't know that he was closing the PR.
I think he is new to open source community :). We will reach him.

Sorry for that !

@Gaspard-Bourgeois
Copy link
Author

I'm sorry, I did'nt know closing meant that

@Julien-Livet Julien-Livet deleted the branch ros-industrial:kinetic-devel January 14, 2022 09:32
@Julien-Livet Julien-Livet deleted the kinetic-devel branch January 14, 2022 09:32
Comment on lines +43 to +44
project_config(MotoRos YRC1000 YRC1u_)
project_config(MotoRos YRC1000u YRC1_)
Copy link
Contributor

Choose a reason for hiding this comment

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

The controller names and suffixes are mixed up here

@@ -0,0 +1,159 @@
#Without Visual Studio support: .vcxproj and linked mps files are automatically generated on the fly by CMake macro
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't understand the purpose of this file.

The other file uses the information already present in the mps file and avoids any duplication. It allows generating build configuration for VS, Makefiles or any other CMake target, that will use the mpbuilder.exe from MotoPlus for Visual Studio, which is useful.

This file just produces the same results, only with the difference of spelling everything out explicitly?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

6 participants