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

FollowMe API feels inconsistent #191

Closed
hamishwillee opened this issue Dec 4, 2017 · 6 comments
Closed

FollowMe API feels inconsistent #191

hamishwillee opened this issue Dec 4, 2017 · 6 comments

Comments

@hamishwillee
Copy link
Collaborator

hamishwillee commented Dec 4, 2017

  • bool set_config (const Config &config) - I would expect this to return a Result type
  • void get_last_location(TargetLocation &last_location) could reasonably be be TargetLocation& get_last_location()
  • If we are checking ack, then void set_curr_target_location(const TargetLocation &location) should return a Result.
  • set_curr_target_location() should be set_current_target_location() or even better set_target_location()
  • There should usually be matching async APIs.
@shakthi-prashanth-m
Copy link
Contributor

@hamishwillee Thanks for reporting this. I will make changes accordingly.

@julianoes julianoes removed their assignment Dec 4, 2017
@julianoes
Copy link
Collaborator

There should usually be matching async APIs.

I have no strong opinion on this one. We can add async/sync APIs as required.

@hamishwillee
Copy link
Collaborator Author

There should usually be matching async APIs.

I have no strong opinion on this one. We can add async/sync APIs as required.

Fine by me. This was just a dump of all the things that seemed odd :-)

shakthi-prashanth-m pushed a commit that referenced this issue Dec 13, 2017
Improved API signature as per issue: #191
@hamishwillee
Copy link
Collaborator Author

This will be fixed by #199

julianoes pushed a commit that referenced this issue Dec 14, 2017
Improved API signature as per issue: #191
@shakthi-prashanth-m
Copy link
Contributor

@hamishwillee shall we close this ?
I believe Follow Me example is not part of this issue. Isn't it ?

@hamishwillee
Copy link
Collaborator Author

Yes, fixed :-)

rt-2pm2 pushed a commit to rt-2pm2/DronecodeSDK that referenced this issue Nov 27, 2018
Improved API signature as per issue: mavlink#191
dlech pushed a commit to dlech/MAVSDK that referenced this issue Jan 14, 2022
Corrected the spelling of Behaviour to Behavior.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants