ethz-asl / asctec_mav_framework

Framework for data aquisition and position control to be used with the highlevel processor of Ascending Technologies helicopters
http://www.ros.org/wiki/asctec_mav_framework
36 stars 40 forks source link

Clean version of asctec_hl_gps #53

Closed marija-p closed 8 years ago

marija-p commented 8 years ago

As per @enricgalceran's request, I've tidied up asctec_hl_gps by introducing a dependency to geodetic_utils. All the frame conversions are now performed using this new header-only package and redundant code has been removed (there was quite a lot of it!). I'm not the original author of this code, but I've also done some obvious re-formatting for readability and Google C++ Style. Let me know if anything else should be changed.

Despite these changes, the functionality of asctec_hl_gps should remain the same. It seems to build and work fine for me.

helenol commented 8 years ago

I think we'd ideally actually like to move everything you guys use out of asctec_hl_gps -> a different repo, since we are basically deprecating this whole repo. We've already gotten rid of all the other dependencies on asctec_mav_framework in other repos (as far as we know), and this is the last stuff remaining. @markusachtelik ? @burrimi ? confirm? This also involves switching over to using asctec_communication_interface / ethzasl_mav_interface / etc. rather than the asctec_mav_framework for your communication with the autopilot, at least once we confirm that ACI is working with the older autopilots.

burrimi commented 8 years ago

@helenol I think it's good to have it in here too, since a lot of people are still using this interface.

enricgalceran commented 8 years ago

@helenol I agree with @burrimi, plus this way we avoid having duplicated GPS conversion code.

enricgalceran commented 8 years ago

BTW @marija-p, I think you are already aware of this but you should prefix your branch names with feature/bugfix/cleanup/something.

helenol commented 8 years ago

@burrimi, @enricgalceran, sounds good to me. :) Code looks much cleaner, thanks a lot for doing this! LGTM. (As soon as I fix the dependency in Jenkins).

helenol commented 8 years ago

test this please

helenol commented 8 years ago

Now will work once https://github.com/ethz-asl/geodetic_utils/pull/1 is merged.

ffurrer commented 8 years ago

lgtm