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

remove the yaw inversion in 3D mode when platform is set to ROVER #3128

Merged
merged 2 commits into from
Apr 27, 2018
Merged

remove the yaw inversion in 3D mode when platform is set to ROVER #3128

merged 2 commits into from
Apr 27, 2018

Conversation

eephyne
Copy link
Contributor

@eephyne eephyne commented Apr 26, 2018

Yaw inversion in not wanted when used for steering in ROVER mode.
This simply deactivate this.
Its a fix to wait until there is a real support for rover, since 3d mode is the only way to use reversable esc.

// Reverse yaw servo when inverted in 3D mode
if (feature(FEATURE_3D) && (rcData[THROTTLE] < rxConfig()->midrc)) {
// Reverse yaw servo when inverted in 3D mode except in ROVER mode
if (feature(FEATURE_3D) && (rcData[THROTTLE] < rxConfig()->midrc) && mixerConfig()->platformType != PLATFORM_ROVER) {
Copy link
Member

Choose a reason for hiding this comment

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

@eephyne maybe it would make more sense to reverse this condition? So then yaw would be reversed only for platform_type multirotor and tricopter? So boats, rovers and other and whatever would not have it? What do you thing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thats indeed a better way to do it. you sure its useful only for multirotor and tricopter ?

Copy link
Member

Choose a reason for hiding this comment

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

I'm not even sure there are 3D Tricopters. If so then only Multirotors would reverse it
@stronnag ? I'm even thinking about decoupling 3D Mode from yaw reversal and renaming it to "Reversible motors" or something. But small steps.

Copy link
Contributor Author

@eephyne eephyne Apr 26, 2018

Choose a reason for hiding this comment

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

shouldn't we add also helicopter ? even if I seriously doubt there is anybody using inav on a helicopter…

@DzikuVx DzikuVx changed the base branch from master to development April 26, 2018 15:29
@flounderscore
Copy link

flounderscore commented Apr 26, 2018 via email

@eephyne
Copy link
Contributor Author

eephyne commented Apr 26, 2018

So multirotor and tricopter are enough then

@DzikuVx DzikuVx added this to the 2.0 milestone Apr 27, 2018
@digitalentity digitalentity merged commit 482025f into iNavFlight:development Apr 27, 2018
@eephyne
Copy link
Contributor Author

eephyne commented May 3, 2018

hum, since my esc burned out after rapidly testing my change, I could not test it in depth. (burn not related).
But I moved to adding another fc to another car (in betaflight because there is no target in inav (FF_PIKOBLX).
I added the same modification and it turn out that I need yaw inversion in reverse BUT, reverse only for stabilization, not for manual steering, so my code is not good but the default one neither.
I did'nt have the time for searching where exactly I need to change something but I will.
Its also showed me another issue, when braking you put the throttle in reverse until the car is stopped, but the fc take that as an inversion. After thinking a bit , the only solution I see for that is to monitor acceleration and do not reverse steering if there is any acceleration at the moment of going below 1500 in throttle.
Since this is a specific behavior to car (maybe boat ? I don't know if they have braking function). Its probably the point where you need a feature for this mode.
I'm not good enought right now to do that, But I'll investigate and try to.
But first I need to add piko_blx target.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants