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

Migration to Catkin build system #75

Closed
wants to merge 2 commits into from
Closed

Migration to Catkin build system #75

wants to merge 2 commits into from

Conversation

boris-il-forte
Copy link

  • deleted manifest.xml
  • updated CMakeLists.txt
  • created package.xml

This patch shuold be tested. It's also neeed to change the e-mail in package.xml, it was choosen a random e-mail to make the package compile with catkin

please consider to merge this patch, or to doing youreself the catkin migration: catkin packages can't depend on rosbuild packages, so all new projects based on Groovy can't use the driver anymore.

- deleted manifest.xml
- updated CMakeLists.txt
- created package.xml

This patch shuold be tested. It's also neeed to change the e-mail in package.xml, it was choosen a random e-mail to make the package compile with catkin
@mani-monaj
Copy link
Member

Thank you very much for the patch @boris-il-forte . I think we need a branch for hydro to add support for catkin based on your patch. I do not have enough experience with catkin. Do you think if it is easy to make an independent catkinized package for ARDroneLib?

@boris-il-forte
Copy link
Author

I think it would be easy... It's just about converting the existent cmakefile in a catkin-compliant cmakelist.txt, that's nothing special, and creating a package.xml with all the correct fields... I can help if is needed...

EDIT:
I need to update my comment, because I answered too quickly.

I made a mistake when I said "update cmake files", because the whole project is based on ARDronelib Make, create a CMakeList.txt is not as straightforward as I thought.

basically I think you could choose two ways, the first is to call "make" or a script from CMakeList, and move the generated targets in the correct folders used by catkin, the second is to rewrite the entire build system using cmake, which itself should not be difficult, but definitely takes some effort.

the second way is definitely the cleanest, the first is a dirty trick to force the use of Make in catkin, which is designed precisely to prevent this.

@lehnerpat
Copy link

👍 for a catkinized branch! I'd very much appreciate if you could maintain/transition to a catkin version of the package for hydro.

I can also confirm that @boris-il-forte's changes in this PR work wonderfully for now. The installation works like before, the only command that needs to be substituted is to use catkin_make instead of rosmake:

  1. clone ardrone_autonomy into $catkinWorkspace/src/ardrone_autonomy
  2. cd $catkinWorkspace/src/ardrone_autonomy
  3. ./build_sdk.sh (and install dependencies if necessary/prompted)
  4. cd $catkinWorkspace
  5. catkin_make
  6. happiness ❤️

@mani-monaj
Copy link
Member

Thank you @boris-il-forte for information and @Nevik for testing. I will merge the changes/update the driver this coming week.

@boris-il-forte
Copy link
Author

any news about the porting?
Now Hydro is out, it would be nice to have a Hydro version for this package... maybe including also the compilation of the sdk...

@mani-monaj
Copy link
Member

any news about the porting?
Now Hydro is out, it would be nice to have a Hydro version for this package... maybe including also the compilation of the sdk...

Parrot was supposed to release its new SDK with Flight Recorder and firmware 2.4.x support in September. My initial plan was to update the build system with the new SDK. I will wait a couple of days more for the parrot. I hope they will release their new SDK soon.

- added rules to install ardrone_driver binary in the correct location specified by --install-space of catkin_make command
@boris-il-forte
Copy link
Author

I realized that there were no rules to properly install ardrone_driver in the correct installation directory when you launch catkin_make - install.

it should now install the driver, so that it is accessible with the command rosrun even if you specify an installation directory with the flag "- install-space"

as always, needs to be tested ...

@boris-il-forte
Copy link
Author

simply if the node doesn't compile without is a build_depend, if it crashes at runtime (or if it's needed some other node tho work correctly) is a run_depend

@boris-il-forte
Copy link
Author

No news about the new parrot SDK?

@mani-monaj
Copy link
Member

Ok, I managed to fix crashes on 2.4.x firmware by updating the SDK to 2.0.1. I have not tested it completely during flights, but at least it works both for 2.3.x and 2.4.x firmwares. I will patch the SDK further to support the new GPS messages. I will move on to working on catkin branch soon.

The code (8c46987) is now on dev-unstable branch.

@kaizadr
Copy link

kaizadr commented Nov 4, 2013

I just tried to catkin_make this package and came across an error. The RecordEnable.srv file in ardrone_autonomy/srv does not exist and yet it is included in the add_service_files section in CMakeLists.txt.

Can anyone help me with this? Thanks

@boris-il-forte
Copy link
Author

Which branch are you using?
If i'm not wrong, the catkinized branch is not yet merged.
when I've catkinized the branch, the RecordEnable.srv was present, and so i've put this file in the list of message to be generated (I've simply "ls" the directory srv and msg, so I've put all the messages and services in the list of things to be generated).

if you have catkinized yourself the branch, and you don't have the .srv file, why don't you just delete it in the CMakeList.txt? it should work...

@mani-monaj
Copy link
Member

@boris-il-forte I finally pushed the new driver based on SDK 2.0.1 to master. Can you please rebase your branch and check if it still works?

@boris-il-forte
Copy link
Author

Ok, I'll try, and eventually fix all the problems by this evening...

@boris-il-forte
Copy link
Author

Ok, work done!
all worked out-of the box after the merge. I should open another pull request? this seemed to be outdated...

EDIT: I've seen that is not possible anymore to update the branch of the pull request, so I've opened a new one. This shuold be closed...

@mani-monaj
Copy link
Member

Follow-up on #79

@mani-monaj mani-monaj closed this Nov 22, 2013
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