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

API improvement: Independent gimbal angle control #4527

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

ibrhm0v
Copy link
Contributor

@ibrhm0v ibrhm0v commented May 17, 2022

Context

Improving pythonclient, rpc and unreal apis to allow independent control of each gimbal angle. This update allows the control of each gimbal angle separately for a given camera. Therefore we won't need to pass a Pose struct for every and every gimbal control action. This will minorly improve performance and increase camera control flexibility. Docs update is also included in this PR

How Has This Been Tested?

Tested through a simple gimbal control python script

@rajat2004
Copy link
Contributor

IMHO it's not really worth it adding extra APIs just for controlling one specific parameter, going this way will lead to explosion in number of APIs. Do you have any numbers for the performance improvement, doubt there would be much difference.
If the current pose API doesn't handle controlling independent angles, maybe it can be enhanced to only apply non-zero control values. But zero pitch or roll is also valid and something which the user wants to apply, maybe an optional argument (like a bitset), indicating which clause the user wants to control? Wdyt

@ibrhm0v
Copy link
Contributor Author

ibrhm0v commented May 23, 2022

I agree with you. But for a gimbal, as most real gimbal controller APIs allow, controlling each angle separately would be useful. Tracking an object during maneuver can be given as an example. This helped me in my application. I thought it would be good to help others too.
By the way your idea also sounds positive to me, advancing the simSetCameraPose by adding a typemask argument to select control of position and each angle would also be good. But might increase complexity for the user. I can update my implementation that way. Your opinions would be appreciated

@ibrhm0v
Copy link
Contributor Author

ibrhm0v commented May 30, 2022

@rajat2004 Any thoughts on this?

@rajat2004
Copy link
Contributor

Sorry, missed the reply. Bitmask seems to be the better option to me atleast. What do you think @zimmy87 @jonyMarino

@jonyMarino
Copy link
Collaborator

Thanks for the contribution and the debate. I think the current implementation is ok. I would refactor the client APIs to get objects with specialized APIs (for example, for cameras). But that is out of the scope of this contribution. I will continue thinking about this.

@jonyMarino jonyMarino changed the base branch from master to main July 17, 2022 13:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants