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.

Sign up for GitHub

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

Jump to bottom

vmount: support absolute angles #10525

Merged
merged 6 commits into from
Jan 6, 2020
Merged

vmount: support absolute angles #10525

merged 6 commits into from
Jan 6, 2020

Conversation

julianoes
Copy link
Contributor

@julianoes julianoes commented Sep 19, 2018 •
edited by AuterionWrikeBot
Loading

The mavlink spec now supports absolute angles as well, see:
mavlink/mavlink#944

@julianoes julianoes requested a review from bkueng September 19, 2018 12:35
@julianoes julianoes changed the title vmount: first step to support absolute angles [WIP] vmount: first step to support absolute angles Sep 19, 2018
LorenzMeier
LorenzMeier previously approved these changes Sep 19, 2018
View reviewed changes
@dagar dagar mentioned this pull request Sep 19, 2018
Closed
@dagar
Copy link
Member

dagar commented Sep 19, 2018

@julianoes how are you testing this? One problem I have with vmount for #10502 is not knowing the other use cases to check for any regressions.

@julianoes
Copy link
Contributor Author

@dagar I haven't tested it yet which is why it's WIP. I'm planning to cherry-pick it for H520 and test it then against the SDK.

bkueng
bkueng reviewed Sep 20, 2018
View reviewed changes
Copy link
Member

@bkueng bkueng left a comment

Choose a reason for hiding this comment

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

Looks good so far.

@stale
Copy link

stale bot commented Jan 20, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@julianoes
Copy link
Contributor Author

@bkueng any thoughts what we should do with this?

@weekly-digest weekly-digest bot mentioned this pull request Jan 27, 2019
Closed
@stale
Copy link

stale bot commented Feb 4, 2019

Closing as stale.

@stale stale bot closed this Feb 4, 2019
@julianoes julianoes reopened this Feb 4, 2019
@stale stale bot removed the Admin: Wont fix label Feb 4, 2019
@julianoes
Copy link
Contributor Author

Note to self, need to work on this.

@weekly-digest weekly-digest bot mentioned this pull request Feb 10, 2019
Closed
@julianoes julianoes mentioned this pull request May 8, 2019
Closed
@julianoes julianoes force-pushed the add-vmount-absolute-angle branch 2 times, most recently from e739260 to 931fda9 Compare July 1, 2019 12:55
@julianoes
Copy link
Contributor Author

Rebased, currently testing and fixing this.

@weekly-digest weekly-digest bot mentioned this pull request Jul 7, 2019
Closed
@stale
Copy link

stale bot commented Sep 29, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@hamishwillee
Copy link
Contributor

@julianoes YOu might want to pin this

@stale stale bot removed the Admin: Wont fix label Oct 1, 2019
@julianoes
Copy link
Contributor Author

This will change anyway when the new gimbal messages come to MAVLink, closing.

@julianoes julianoes closed this Oct 1, 2019
@julianoes julianoes deleted the add-vmount-absolute-angle branch October 1, 2019 08:34
@julianoes julianoes restored the add-vmount-absolute-angle branch December 9, 2019 10:53
@julianoes julianoes reopened this Dec 9, 2019
@julianoes
Copy link
Contributor Author

Re-opening this because it's needed after all.

@julianoes julianoes force-pushed the add-vmount-absolute-angle branch from 931fda9 to 763012d Compare December 9, 2019 12:23
@julianoes julianoes changed the title [WIP] vmount: first step to support absolute angles vmount: support absolute angles Dec 9, 2019
@julianoes julianoes force-pushed the add-vmount-absolute-angle branch from 763012d to d2bbeb6 Compare December 11, 2019 13:23
@julianoes
Copy link
Contributor Author

@bkueng would be great if you could review this again some time.

bkueng
bkueng reviewed Dec 12, 2019
View reviewed changes
Copy link
Member

@bkueng bkueng left a comment

Choose a reason for hiding this comment

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

Looks good. Can you cleanup the commits (there's a style fix)?

src/modules/vmount/input_mavlink.cpp Outdated
param = vehicle_command.param7;
}

if (int(param) == 0) {
Copy link
Member

Choose a reason for hiding this comment

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

I guess you need to round or add the 0.5 here

Suggested change
if (int(param) == 0) {
if (int(param+0.5f) == 0) {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thx.

src/modules/vmount/input_mavlink.cpp Outdated
for (int i = 0; i < 3; ++i) {
float param;

if (i == 0) {
Copy link
Member

Choose a reason for hiding this comment

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

You could do: float params[] = { vehicle_command.param5, vehicle_command.param6, vehicle_command.param7 }; to keep this shorter.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice, I'll do something like that.

@julianoes julianoes force-pushed the add-vmount-absolute-angle branch from 4a6a37e to 266e244 Compare December 16, 2019 12:24
@julianoes
Copy link
Contributor Author

@bkueng if you could have another quick look over this, that would be great. And then I can probably merge it.

bkueng
bkueng previously approved these changes Dec 20, 2019
View reviewed changes
Copy link
Member

@bkueng bkueng left a comment

Choose a reason for hiding this comment

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

Can you squash the style fix commit?

@weekly-digest weekly-digest bot mentioned this pull request Dec 22, 2019
Closed
@julianoes
vmount: first step to support absolute angles
e9aafc1 
The mavlink spec now supports absolute angles as well, see:
mavlink/mavlink#944
@julianoes
vmount: set all DO_MOUNT_CONFIGURE params
1e11d55 
We previously did not set param2 to param7 of DO_MOUNT_CONFIGURE.
@julianoes
vmount: fix implicit double promotion
2cf8ee5
@julianoes
vmount: fix input for test command
f747d08 
It is most convenient to use a yaw angle relative to body for testing.
Also, we set stabilize_axis to true for testing.
@julianoes
vmount: properly initialize last_active
801ec18 
If last_active is initialized at 0 it means that input via mavlink is
already enabled but that's generally not the case.
@julianoes
vmount: fix float casting, simplify loop
e187491
@julianoes julianoes force-pushed the add-vmount-absolute-angle branch from 266e244 to e187491 Compare January 6, 2020 10:22
@julianoes
Copy link
Contributor Author

@bkueng style commit is squashed, let's wait for CI.

@bkueng bkueng merged commit 25ff013 into master Jan 6, 2020
@bkueng bkueng deleted the add-vmount-absolute-angle branch January 6, 2020 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bkueng bkueng bkueng approved these changes

@LorenzMeier LorenzMeier LorenzMeier left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@julianoes @dagar @hamishwillee @bkueng @LorenzMeier