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

Corrections on DO_SET_SERVO mission item #4965

Closed
wants to merge 2 commits into from
Closed

Corrections on DO_SET_SERVO mission item #4965

wants to merge 2 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Jul 2, 2016

Hello,
I have modified default auxiliary mixer to accept input from actuators group 2
Next i added bootlog.txt as output into rc.interface to have more visible mixers loading process when user try to upload own mixer
Last change - adding DO_SET_SERVO mission item check in navigator( before only first mission item are checked) and correction of limits for id of AUX outputs ( from 0...5 to 1...6)
Changes are tested in flight today for all 6 AUX channels.
Regards,
Michail.

More visible mixers loading process with output to bootlog.txt
adding DO_SET_SERVO mission item check in navigator( before only first mission item are checked)
}
/* check only items with valid lat/lon */
else if (isPositionCommand(mission_item.nav_cmd)) {
/* Check non navigation item */
Copy link
Member

Choose a reason for hiding this comment

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

Why is this check now present 2 times?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hello Lorenz,
I have sent the pull request from my new github account "px4user".
In your comment we have a check of output first and next the check of desired PWM value.
But generally in module mission_feasibility_checker.cpp we have function checkMissionItemValidity where we check all mission items
Als we have function check_dist_1wp where some body have initially added check of DO_SET_SERVO mission item. From my point of view it can be excluded. But as is not me who have added this i prefered to keep it.
Regards,
Michail.

Copy link
Member

Choose a reason for hiding this comment

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

I would prefer to have the check at a single location.

@LorenzMeier
Copy link
Member

Thanks for the update!

@ghost
Copy link
Author

ghost commented Jul 11, 2016

Hello, Lorenz,
I have removed double check and tested yesterday in flight.
Tested for normal work and for critical situations when i try to enter wrong values, when i put this mission item at the start or end of mission.
Regards,
Michail.

@fpvaspassion
Copy link
Contributor

Hello? Lorenz,
Do you need some additional actions from my side, if not - when you expect to push it to master?
Michail.

@bkueng
Copy link
Member

bkueng commented Jul 18, 2016

Lorenz is currently travelling, he'll be back by the end of the month

@fpvaspassion
Copy link
Contributor

Hi,
Thanks for this information, will wait.
Regards.

@fpvaspassion
Copy link
Contributor

Hello, Lorenz!
Can you have a look to this PR?
Thanks in advance.
Michail.

@LorenzMeier
Copy link
Member

Navigator change looks good, the mixer change is difficult - you can now have conflicting inputs on the payload.

I would like to understand the issue you were trying to solve - is the issue that manual pass-through in your case is not a good thing? It seems like we would need a payload controller arbitrating between manual and automatic passthrough mode.

@LorenzMeier
Copy link
Member

@thedevleon I think you also fixed the pass-through mixer in your PR. I'd like to get that onto master before the release.

S: 3 4 10000 10000 0 -10000 10000

# AUX5 channel (select servo id 5 in mission item)
Copy link
Member

Choose a reason for hiding this comment

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

I need to check if this can conflict with camera triggering.

@fpvaspassion
Copy link
Contributor

Hi!
Finally i search to manipulate payload in mission, and you propose to use other subsrciptions? I do not understand where we can have a conflict.
My idea of mixers modification is following - you have a camera shooter on copter. When you do preflight checks you can check functionnement of shooter with AUX switch on RC radio, when you fly the mission to made an aerial map - you use mission items 183 to made shoot a pictures.
But always users can load other mixers if it will be some specific airframes or configuratoins.
Regards,
Michail.

@fpvaspassion
Copy link
Contributor

Ok, i see - it can confict with trigger module, i never checked with control group this module use....

@PX4BuildBot
Copy link
Collaborator

Can one of the admins verify this patch?

@TSC21
Copy link
Member

TSC21 commented Aug 16, 2017

@fpvaspassion status?

@dagar
Copy link
Member

dagar commented Jan 17, 2018

I'm not sure what happened here, but is there still interest in this PR? If so it badly needs a rebase.

@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.

@stale
Copy link

stale bot commented Apr 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.

@stale
Copy link

stale bot commented May 4, 2019

Closing as stale.

@stale stale bot closed this May 4, 2019
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.

6 participants