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

Adding FollowMe plugin #142

Merged
merged 100 commits into from
Dec 1, 2017
Merged

Adding FollowMe plugin #142

merged 100 commits into from
Dec 1, 2017

Conversation

shakthi-prashanth-m
Copy link
Contributor

@shakthi-prashanth-m shakthi-prashanth-m commented Oct 27, 2017

Highlights of the FollowMe PR:

  • FollowMe::start() changes Flight mode to FollowMe
  • For testing, we can set Follow info FollowMe::set_follow_info(const FollowMe::FollowInfo &info) which involves initial position of Ground control station, etc.
  • FollowMe behavior can be configured using FollowMe::set_config(const FollowMe::Config &config) as per PX4 FollowMe configuration.
  • FollowMe plugin periodically sends GCS location updates to Vehicle
  • FollowMe::stop() leaves the device in Loiter

Shakthi Prashanth M and others added 5 commits October 27, 2017 13:13
1. Changes flight mode to 'Follow Me'
2. Periodic emition of motion reports (As of now hard-coded) to PX4.
3. Added integration tests for FollowMe
4. Added FolloMe example.
1. Changes flight mode to 'Follow Me'
2. Periodic emition of motion reports (As of now hard-coded) to PX4.
3. Added integration tests for FollowMe
4. Added FolloMe example.
1. FollowMe::MotionReport is the class that holds motion info of the GCS.
2. Exported FollowMe::set_motion_report() method for use by example.
3. Added FollowMe::stop()
4. Added overloaded version: FollowMe::start(const MotionReport &mr)
3. FollowMe::result_str() now returns std::string.
4. FollowMe example does start FollowMe mode and stops it after 20 seconds.
5. Fixed minor build errors.
@coveralls
Copy link

Coverage Status

Coverage remained the same at 48.634% when pulling 559d389 on follow_me into 97d98b9 on develop.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 48.634% when pulling 0710365 on follow_me into 97d98b9 on develop.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 48.634% when pulling 84a5ec4 on follow_me into 97d98b9 on develop.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 48.634% when pulling a71893a on follow_me into 97d98b9 on develop.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 48.634% when pulling df5f81e on follow_me into 97d98b9 on develop.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 48.634% when pulling ce3ee69 on follow_me into 97d98b9 on develop.

Copy link
Collaborator

@julianoes julianoes left a comment

Choose a reason for hiding this comment

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

I left some comments inline.

Also, one thing to look into, is probably the orientation of the drone versus the target. See here for more info:
https://github.com/PX4/Firmware/blob/master/src/modules/navigator/follow_target.cpp and
https://docs.px4.io/en/flight_modes/follow_me.html#configuration

This is going to be fun when done :)

@@ -0,0 +1,39 @@
cmake_minimum_required(VERSION 2.8.12)

project(follow_target)
Copy link
Collaborator

Choose a reason for hiding this comment

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

CMakeLists.txt for examples get a lot easier once this is in: https://github.com/dronecore/DroneCore/pull/130/files#diff-b33c729b1001aff667a2a60dcf6be327R6.

/**
* @file follow_target.cpp
* @brief Example that switches Vehicle mode to FollowMe.
* @author Author: Shakthi Prashanth M <shakthi.prashanth.m@intel.com>
Copy link
Collaborator

Choose a reason for hiding this comment

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

In general, I recommend doing integration tests when adding a new features. The reasons are:

  • We need integration tests for all the functionality anyway, so we might as well do it while implementing it.
  • The example should only cover a simple part of the API to quickly get a developer up to speed.
  • It is easier to compile and run it because you don't always need to install the library, and then build the example and link it.
  • You can use the internal LogDebug() statements.

I propose you move this over to an integration test and add an example at the very end when everything is working.

Oh, I see now you also added an integration test. I would suggest you keep developing there and create a small example at the end.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree. Done.

mission_change_speed
mission_survey
gimbal
mission
followme_target
Copy link
Collaborator

Choose a reason for hiding this comment

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

indenting.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.


using namespace dronecore;

TEST_F(SitlTest, FollowMeTargetTest)
Copy link
Collaborator

Choose a reason for hiding this comment

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

No Test in name needed, just FollowMe is fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done


std::this_thread::sleep_for(std::chrono::seconds(5));

FollowMe::Result followme_result = device.followme().start();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is it followme but the files are called follow_me? Can you make it consistent?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think, it is taking this name from classname. It won't be proper if I name plugin class name as Follow_Me. So, I felt follow_me is intuitive than followme.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yer, I need to fix it.

// Ideally GPS info should come from a platform-specific framwork.

// update current location coordinates
_motion_report.lat_int += 10;
Copy link
Collaborator

Choose a reason for hiding this comment

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

It would be nice to have an example that does a box or a circle.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will figure out the coordinates for bounding box and then add.

using namespace std::chrono;

auto end = system_clock::now();
auto elapsed_seconds = system_clock::to_time_t(end) - system_clock::to_time_t(_start);
Copy link
Collaborator

Choose a reason for hiding this comment

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

For everything with time please re-use the time functionality from global_include.h. This way we can also mock time for testing, and keep it consistent.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

auto end = system_clock::now();
auto elapsed_seconds = system_clock::to_time_t(end) - system_clock::to_time_t(_start);

float vel[3] = { _motion_report.vx, _motion_report.vy, 0.f };
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would probably make these const float vel[3] ... since you're not going to change them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

custom_state);

std::cout << __func__ <<
((_parent->send_message(msg)) ? ("--------------------> sent follow_target")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please use LogDebug() etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done


TEST(FollowMeImpl, NoTest)
{
ASSERT_TRUE(true);
Copy link
Collaborator

Choose a reason for hiding this comment

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

This would be a unit test but if there are none, you can probably leave the file out.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed, as it is redundant

Copy link
Collaborator

Choose a reason for hiding this comment

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

Still looks to be here to me :-)

@LorenzMeier
Copy link
Member

Firmware fix merged.

};

/**
* @brief Motion report info which will be emitted peridically to the Vehicle
Copy link
Collaborator

Choose a reason for hiding this comment

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

peridically => periodically

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

*/
struct MotionReport {
// set to home position used by PX4.
int32_t lat_int = static_cast<uint32_t>(47.3977419 *
Copy link
Collaborator

Choose a reason for hiding this comment

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

if this is home position, name it appropriately => home_latitude. No need to put "type" in name. If this is public API then have a doc string output with @brief tag.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is not public API now.

};

/**
* @brief Motion report info which will be emitted peridically to the Vehicle
Copy link
Collaborator

@hamishwillee hamishwillee Oct 29, 2017

Choose a reason for hiding this comment

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

So what is the purpose of this? You're sending current motion of vehicle? Why does a user of the API need to know this/set this - shouldn't it be autopopulated? Or are we sending the expected behaviour of the vehicle? In that case, I would again expect that we'd simplify what we want the vehicle to do.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@hamishwillee You're right. This should be auto-populated. But for testing purpose, we need to export Position (lat, long, alt) to API users. And, this structure will be part of impl and is auto-populated.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@shakthi-prashanth-m (& @julianoes ) Just to be clear, are you saying that this will move to be internal?, and you'll test it via the internal API?
For me the API should look something like start(), stop(), setFollowInfo(FollowMe::FollowInfo info), getFollowInfo()
Where FollowInfo would be info to specify the relative location that the follower should adopt. This might be a co-ordinate or "standard position + height" (front, back, front left, back left, height) or behaviour (e.g. circle me).

I don't think anything else is needed, though possibly a string for printing output if we use standard positions (and hence have an enum)

Copy link
Collaborator

Choose a reason for hiding this comment

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

I agree with @hamishwillee. We don't need to internally check GPS. We just expose it via an API.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As per https://docs.px4.io/en/flight_modes/follow_me.html QGroundControl doesn't prompt user for position. It captures current position & other GPS related details using platform-specific framework and sends to device. So, in our case we're setting position only for testing purpose.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@shakthi-prashanth-m @julianoes

So, in our case we're setting position only for testing purpose.

No no no no. That is horrible. Nothing to do with testing should be in a public API file. Making it conditional does not make it any less ugly. If you need to have some sort of test stuff, then have it in an implementation file and test that file.

So I had another look at the mode docs. Updating my comment above in line with what PX4 does:

The API should be start(), stop(), setFollowInfo(FollowMe::FollowInfo info), getFollowInfo()
Where FollowInfo would be and object that captures all the config params here. You would then set/get the params as needed to drive the mode. You would have an enum and a string converter for NAV_FT_FS

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree about FollowInfo parameters. But somehow need to mock GPS position of the GCS and send periodic updates to device. It could be done internally, without a user API. As per FollowMe mode description here, GPS is needed. It's mentioned in the top,

By using GPS and other positioning information a multicopter is able to automatically yaw to face and follow a user at a specified position and distance. While in this mode no user input is required.
This mode requires GPS.

As discussed with @hamishwillee in slack, we can do as below:

  1. Cleanup public header file follow_me.h and handle mocking position stuff in follow_me_impl.h.
  2. Export public APIs for set/get FollowInfo
  3. To use CallEveryHandler instead of TimeoutHandler, as per @julianoes suggestion.

Copy link
Collaborator

Choose a reason for hiding this comment

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

No, why would you mock this position. Just feed it to DroneCore via the public API.

Copy link
Collaborator

Choose a reason for hiding this comment

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

No, why would you mock this position. Just feed it to DroneCore via the public API.

@julianoes Perhaps I'm dim, but what public API does DroneCore have for setting its ownposition? If such an API exists great - I wasn't aware we had platform-independent methods of getting position of dronecore in place.

@shakthi-prashanth-m
Copy link
Contributor Author

[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from SitlTest
[ RUN      ] SitlTest.FollowMe
Not autostarting SITL (use AUTOSTART_SITL=1)
[07:39:23|Info ] New device on: 127.0.0.1:14557 (udp_connection.cpp:210)
FlightMode: Takeoff
FlightMode: Takeoff
FlightMode: Takeoff
FlightMode: Takeoff
FlightMode: Takeoff
FlightMode: FollowMe
FlightMode: FollowMe
FlightMode: FollowMe
FlightMode: FollowMe
FlightMode: FollowMe
FlightMode: FollowMe
FlightMode: FollowMe
FlightMode: FollowMe
FlightMode: FollowMe
FlightMode: FollowMe
FlightMode: Hold
FlightMode: Hold
[       OK ] SitlTest.FollowMe (23078 ms)
[----------] 1 test from SitlTest (23078 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test case ran. (23078 ms total)
[  PASSED  ] 1 test.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 48.634% when pulling 82e39a2 on follow_me into 694b0a9 on develop.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 48.634% when pulling 4a16437 on follow_me into 97d98b9 on develop.

Shakthi Prashanth M added 2 commits November 3, 2017 19:18
1. Changes flight mode to 'Follow Me'
2. Periodic emition of motion reports (As of now hard-coded) to PX4.
3. Added integration tests for FollowMe
4. Added FolloMe example.
@shakthi-prashanth-m
Copy link
Contributor Author

@julianoes you can review now. Thanks.

1. Set location update frequency: 1Hz
2. Other minor changes
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.07%) to 17.406% when pulling cd9d9e8 on follow_me into 61dce08 on develop.

Copy link
Collaborator

@julianoes julianoes left a comment

Choose a reason for hiding this comment

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

I made a few more small comments but I think this is pretty much good to go.

FollowMe::TargetLocation last_location;
device.follow_me().get_last_location(last_location);

std::cout << "FlightMode: " << Telemetry::flight_mode_str(flight_mode)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think for integration tests we could re-use LogInfo(), etc. but we can do that in a later pull request because it's currently not consistent.

{
std::cout << "Current FollowMe configuration of the device" << std::endl;
std::cout << "---------------------------" << std::endl;
std::cout << "Min Height: " << config.min_height_m << "m" << std::endl;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Units! Yay :)

case FollowMe::Config::FollowDirection::NONE:
return "None";
}
return nullptr; // to please compiler not to warn
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think alternatively you could add the default:.

return "Timeout";
case Result::NOT_ACTIVE:
return "FollowMe is not active";
case Result::UNKNOWN:
Copy link
Collaborator

Choose a reason for hiding this comment

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

When you omit the break or return you need to add a comment // FALLTHROUGH, otherwise newer compilers will warn.

constexpr static const float MIN_RESPONSIVENESS = 0.f; /**< @brief Min responsiveness. */
constexpr static const float MAX_RESPONSIVENESS = 1.0f; /**< @brief Max responsiveness. */

float min_height_m = DEFAULT_HEIGHT_M; /**< @brief Min follow height above home, in meters. */
Copy link
Collaborator

Choose a reason for hiding this comment

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

What about just setting the number here instead of having the #define? Do you think it is needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, it is not used anywhere. Initially I had used these constants for future use, if we need default values, etc. It seems we don't require them. I shall remove them and use literals.

{
// If the target's latitude is NAN, we assume that location is not set.
// We assume that mutex was acquired by the caller
return std::isfinite(_curr_target_location.latitude_deg);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Yep, I think that's fine.

}

{
_mutex.lock();
Copy link
Collaborator

Choose a reason for hiding this comment

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

For the case where you exit early with a return you can also use the std::lock_guard which unlocks when it goes out of scope.

@shakthi-prashanth-m
Copy link
Contributor Author

@julianoes Thanks for the review. Next commit should resolve your comments + using new doxygen tags.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.07%) to 17.406% when pulling ea1f808 on follow_me into 0d87dbe on develop.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.07%) to 17.406% when pulling 68abac1 on follow_me into 0d87dbe on develop.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.07%) to 17.406% when pulling aba29e9 on follow_me into d4f7ef9 on develop.

@julianoes
Copy link
Collaborator

Clang is complaining :(

/Users/travis/build/dronecore/DroneCore/integration_tests/follow_me.cpp:59:75: error: implicit conversion increases floating-point precision: 'float' to 'double' [-Werror,-Wdouble-promotion]

    device.follow_me().set_curr_target_location({47.39768399, 8.54564155, 0.f, 0.f, 0.f, 0.f});

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.07%) to 17.406% when pulling 815f6da on follow_me into d358271 on develop.

@shakthi-prashanth-m
Copy link
Contributor Author

@julianoes, Yeah corrected it. It done now.

@shakthi-prashanth-m
Copy link
Contributor Author

Thanks @julianoes and @hamishwillee for conducting detailed review of this pull request. 💯

@julianoes
Copy link
Collaborator

julianoes commented Dec 1, 2017

@shakthi-prashanth-m thanks for your work, I'm just rebasing this and fixing the conflicts. Doing a squash-merge instead.

@julianoes julianoes merged commit b24022d into develop Dec 1, 2017
@julianoes julianoes deleted the follow_me branch December 1, 2017 16:26
@hamishwillee
Copy link
Collaborator

@shakthi-prashanth-m Awesome this is in. Now that it is all well understood, do you plan on creating an example?

@shakthi-prashanth-m
Copy link
Contributor Author

@hamishwillee FollowMe example should use either of Android/Apple/Windows platform. As of now, we've simulated in integration_tests/follow_me.cpp::FollowMeMultiLocationWithConfig where it feeds GPS Location to FollowMe plugin. To build an example in one of the above platforms, we need Language bindings in-place.

rt-2pm2 pushed a commit to rt-2pm2/DronecodeSDK that referenced this pull request Nov 27, 2018
* Implementation of Follow Me mode.

* Adding FollowMe plugin

1. Changes flight mode to 'Follow Me'
2. Periodic emition of motion reports (As of now hard-coded) to PX4.
3. Added integration tests for FollowMe
4. Added FolloMe example.

* Adding FollowMe plugin

1. Changes flight mode to 'Follow Me'
2. Periodic emition of motion reports (As of now hard-coded) to PX4.
3. Added integration tests for FollowMe
4. Added FolloMe example.

* follow_me: ran fix_style

* Refactored FollowMe plugin.

1. FollowMe::MotionReport is the class that holds motion info of the GCS.
2. Exported FollowMe::set_motion_report() method for use by example.
3. Added FollowMe::stop()
4. Added overloaded version: FollowMe::start(const MotionReport &mr)
3. FollowMe::result_str() now returns std::string.
4. FollowMe example does start FollowMe mode and stops it after 20 seconds.
5. Fixed minor build errors.

* follow_me: fixed type conversion error.

* follow_me: fixed error "implicit conversion from ‘float’ to ‘double’"

* follow_me: ran `make fix_style`

* followme: hope this should fix "conversion from `double` to `uint64_t`"

* converted elapsed time to milliseconds. some minor change in example.

* Implementation of Follow Me mode.

* Adding FollowMe plugin

1. Changes flight mode to 'Follow Me'
2. Periodic emition of motion reports (As of now hard-coded) to PX4.
3. Added integration tests for FollowMe
4. Added FolloMe example.

* Adding FollowMe plugin

1. Changes flight mode to 'Follow Me'
2. Periodic emition of motion reports (As of now hard-coded) to PX4.
3. Added integration tests for FollowMe
4. Added FolloMe example.

* follow_me: ran fix_style

* Refactored FollowMe plugin.

1. FollowMe::MotionReport is the class that holds motion info of the GCS.
2. Exported FollowMe::set_motion_report() method for use by example.
3. Added FollowMe::stop()
4. Added overloaded version: FollowMe::start(const MotionReport &mr)
3. FollowMe::result_str() now returns std::string.
4. FollowMe example does start FollowMe mode and stops it after 20 seconds.
5. Fixed minor build errors.

* follow_me: fixed type conversion error.

* follow_me: fixed error "implicit conversion from ‘float’ to ‘double’"

* follow_me: ran `make fix_style`

* followme: hope this should fix "conversion from `double` to `uint64_t`"

* converted elapsed time to milliseconds. some minor change in example.

* integration_tests: Gimbal entry was missed accidently. Restored now.

* follow_me: Resolved most of the review comments.

1. Removed example follow_me for now.
2. Integrated with other flight modes
3. Unit test inside follow_me plugin is removed as it is redundant.
4. FOLLOW_ME_TESTING flag is used for testing

* follow_me: fixed ununsed const variable

* ran `make fix_style`

* follow_me: Refactoring

* follow_me: making drone to oscillate around a mocked GCS position

* core: added get elapsed time in msec: needed for mavlink commands.

Added FollowMe configuration

* Restored integration_tests/CMakeLists.txt

* integration_tests: follow_me changes

* travis-ci fix: included <array>

* Resolved review comments.

1. Fixed documentation issues
2. Added to_str() method to get Follow dir.
3. Removed method definitions from FollowMe class to keep consistent interface with other plugins.
4. Replaced C++11 style array by normal array to fix Windows build error.

* integration_tests: indentation

* follow_me: fixed Windows build error: conversion from 'double' to 'int32_t'

* Fixed Windows build error, removed repeated timer registration.

* PX4 param used for FollowMe Min Height was misspelled. Corrected it.

* Implementation of Follow Me mode.

* Adding FollowMe plugin

1. Changes flight mode to 'Follow Me'
2. Periodic emition of motion reports (As of now hard-coded) to PX4.
3. Added integration tests for FollowMe
4. Added FolloMe example.

* Adding FollowMe plugin

1. Changes flight mode to 'Follow Me'
2. Periodic emition of motion reports (As of now hard-coded) to PX4.
3. Added integration tests for FollowMe
4. Added FolloMe example.

* follow_me: ran fix_style

* Refactored FollowMe plugin.

1. FollowMe::MotionReport is the class that holds motion info of the GCS.
2. Exported FollowMe::set_motion_report() method for use by example.
3. Added FollowMe::stop()
4. Added overloaded version: FollowMe::start(const MotionReport &mr)
3. FollowMe::result_str() now returns std::string.
4. FollowMe example does start FollowMe mode and stops it after 20 seconds.
5. Fixed minor build errors.

* follow_me: fixed type conversion error.

* follow_me: fixed error "implicit conversion from ‘float’ to ‘double’"

* follow_me: ran `make fix_style`

* followme: hope this should fix "conversion from `double` to `uint64_t`"

* converted elapsed time to milliseconds. some minor change in example.

* Refactored FollowMe plugin.

1. FollowMe::MotionReport is the class that holds motion info of the GCS.
2. Exported FollowMe::set_motion_report() method for use by example.
3. Added FollowMe::stop()
4. Added overloaded version: FollowMe::start(const MotionReport &mr)
3. FollowMe::result_str() now returns std::string.
4. FollowMe example does start FollowMe mode and stops it after 20 seconds.
5. Fixed minor build errors.

* follow_me: Resolved most of the review comments.

1. Removed example follow_me for now.
2. Integrated with other flight modes
3. Unit test inside follow_me plugin is removed as it is redundant.
4. FOLLOW_ME_TESTING flag is used for testing

* follow_me: fixed ununsed const variable

* ran `make fix_style`

* follow_me: Refactoring

* follow_me: making drone to oscillate around a mocked GCS position

* core: added get elapsed time in msec: needed for mavlink commands.

Added FollowMe configuration

* integration_tests: follow_me changes

* travis-ci fix: included <array>

* Resolved review comments.

1. Fixed documentation issues
2. Added to_str() method to get Follow dir.
3. Removed method definitions from FollowMe class to keep consistent interface with other plugins.
4. Replaced C++11 style array by normal array to fix Windows build error.

* integration_tests: indentation

* follow_me: fixed Windows build error: conversion from 'double' to 'int32_t'

* Fixed Windows build error, removed repeated timer registration.

* PX4 param used for FollowMe Min Height was misspelled. Corrected it.

* follow_me: Added doxygen tags for See also. API name changes for responsivess param.

* follow_me: fixed shadow member error.

* Major changes.

1. Simplified FollowMe plugin interface.
2. Addressed review comments.

* follow_me: Added range for responsiveness

* follow_me: Added doxygen comments for FollowMe config

* follow_me: Fixes Travis-CI error: 'FollowInfo' in 'class dronecore::FollowMe' does not name a type

* follow_me: removed `last_request_time` tentatively

* follow_me: Fixed type conversion error during Windows build

* follow_me: minor changes related to FOLLOW_ME_TESTIING

* follow_me: Major changes

1. Removed follow test compile-time flags.
2. For Mock testing, application can register Follow target info callback,
   which will be used by FollowMe plugin to get motion info for sending the same to device.
3. To test on the real mobile devices, application must not register any callbacks.
   FollowMe plugin shall internally subscribes to platform-specific framework to get motion info.
4. It is optional to register follow target info callback.
5. Changed return type of Telemetry::flight_mode_str() to std::string
6. Integration test feeds Fake spiral path mocking GCS position movement for testing.

* follow_me: Added API for de-registering follow target callback.

* follow_me: changes

1. Removed method that obtains Follow target info from underlying platform.
2. Renamed pos_std_dev to eph, epv in FollowMe::FollowTargetInfo.
3. Added detailed note how to register follow target info callback.
4. Renamed callback member name in class FollowMeImpl.
4. Included Error handling to verify callback registration in FollowMe::start().

* follow_me: Fixed doxygen error: 'un-documented param'

* follow_me: Removed callback mechanism.

When there's a change in GPS location of the target,
it is set using FollowMe::set_curr_target_location(); this will
trigger FollowMe to send the current position to the Vehicle.

* Added follow_me at the end

* follow_me: changes below

1. Addressed doxygen review comments.
2. User can set target location similar to Offboard plugin.
3. other minor changes.

* follow_me: changes below

1. integrated with cmake plugin name changes
2. Added FollowMe::get_last_location() for app user's convenience
	to know the most recent location of the target.
3. Necessary changes in follow_me integration test.

* follow_me: ran fix_style, fixed type conversion of float to double issue.

* follow_me: Below changes

1. Undocumented error
2. Removed @sa, @note doxygen tags. These are generating error during doc generation.

* follow_me: changes

1. Set location update frequency: 1Hz
2. Other minor changes

* follow_me: Resolved review comments. Minor changes in integration tests.

* follow_me: Doxygen tags @sa, @note, list items used.

* follow_me: fixed clang error: implicit conversion float to double
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.

5 participants