-
-
Notifications
You must be signed in to change notification settings - Fork 510
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 Follow me example #217
Conversation
void FakeLocationProvider::request_location_updates(location_callback_t callback) | ||
{ | ||
location_callback_ = callback; | ||
timer_.async_wait(boost::bind(&FakeLocationProvider::compute_next_location, this)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you really really need boost for this? I tried to keep everything to plain C++11, if at all possible, I'd vote to do this without boost.
I mean do you actually need to do it async? Or could you just use a loop where you set the location?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@julianoes I really like the async approach - it is far more realistic than just using a loop in the example. Is there something other than boost that could be used here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In my opinion the example doesn't need to show how you could/should build this with async. It should only show how to use the DroneCore API. The simpler the better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@julianoes A simple example in this case can show how the API is called, but now how it should be used. I think the way it is now will save readers time and energy. Your call though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the review @julianoes @hamishwillee. As discussed earlier our example should help application developers including Android, Windows, etc; I simulated approximate layout for the same. I tried simulating Android Location Provider: requestLocationUpdates
.
Even Apple and Windows use this style. As mentioned earlier, I thought using Boost is portable way. I will try to check whether it can be done without Boost.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be good if it could set a Config as well?
Otherwise I really like this.
@hamishwillee Sure, I can. I think Min height is most obvious among the Follow Me configuration. What do you think ? |
Yes. Perhaps even better, both that and the FollowDirection? |
@hamishwillee I added in the example. |
@shakthi-prashanth-m if you rebase this on origin/develop and then force push I can merge it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure why the. gitmodules and CMakeLists.txt changes are in this PR. Otherwise this is good to go in with the comment. Thanks @shakthi-prashanth-m.
1. Added methods for error handling. 2. Improved periodic location update logic. 3. Removed redundant header `cstdint`. 4. Removed log from FollowMe plugin. 5. Improved overall FollowMe example.
This change uses Boost APIs for registering with `FakeLocationProvider`. Removed location generator logic in example. Also, adds stop Follow Me before landing.
Experimental changes in Follow Me integration test was accidentally committed. This commit restore original.
Removed unused variables `curr_direction_s` & `new_direction_s` in `FollowMeImpl::receive_param_follow_direction()`.
This is done for auto-deletion of the location object when control goes out of scope.
1. Added methods for error handling. 2. Improved periodic location update logic. 3. Removed redundant header `cstdint`. 4. Removed log from FollowMe plugin. 5. Improved overall FollowMe example.
This change uses Boost APIs for registering with `FakeLocationProvider`. Removed location generator logic in example. Also, adds stop Follow Me before landing.
Experimental changes in Follow Me integration test was accidentally committed. This commit restore original.
71b9a2a
to
3f1f06e
Compare
@julianoes Sorry |
Add set actuator
This adds Follow Me example.
Adds
FakeLocationProvider
class which mocks platform-specific location provider. It uses Boost Asio for sending periodic report of fake location update. [Note: Boost is chosen for portability across major platforms]Example registers with
FakeLocationProvider
for location updates and sends them to Follow Me plugin.Fixes naming issue in Follow Me plug-in implementation.
Addresses FollowMeImpl::set_config() returns immediately though Device didn't confirm #221
Note: Algorithm that generates latitude, longitude to make drone revolve is rudimentary. As this just serves as an example, it should be fine. If there's a better algorithm, please feel free to replace it.
Thanks