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

Rover: give sailboat motor sailing capability #11423

Open
wants to merge 18 commits into
base: master
from

Conversation

@IamPete1
Copy link
Contributor

commented May 27, 2019

This is quite a big change to sailboat, it gives functionality to use both sail and motor. This gives the sailboat functionality to act as a sailboat only or as traditional rover. This requires the addition of some new parameters that were previously shared.

This adds two new aux functions, one to select the motoring state:
1: never motor
2: enable motoring, the code decides based on wind speed
3: always motor

The second is a aux throttle, this is used in manual mode so 'main' throttle can be used for sail and the 'aux' throttle for motor. Or the other way with a options parameter. The aux throttle can also be used to limit the maximum auxiliary throttle.

Requires #11408 to test in SITL.

@IamPete1 IamPete1 requested review from tridge and rmackay9 May 27, 2019

@IamPete1 IamPete1 force-pushed the IamPete1:motor_sail branch 2 times, most recently from b6d500a to 9d2f0b2 May 28, 2019

@IamPete1

This comment has been minimized.

Copy link
Contributor Author

commented Jun 2, 2019

will need a patch for this #11466 once it goes in, or the other way if this gets in first

APMrover2/sailboat.cpp Outdated Show resolved Hide resolved
@rmackay9

This comment has been minimized.

Copy link
Contributor

commented Jun 3, 2019

So, I have to wonder if we should perhaps move the throttle control back to channel 3 (input and output) and use a new input for the mainsail.

@peterbarker

This comment has been minimized.

Copy link
Contributor

commented Jun 3, 2019

@IamPete1 IamPete1 added the WIP label Jun 7, 2019

@IamPete1 IamPete1 force-pushed the IamPete1:motor_sail branch from 9d2f0b2 to ff82b51 Jun 7, 2019

@IamPete1 IamPete1 force-pushed the IamPete1:motor_sail branch from ff82b51 to 9b4ca4d Jul 20, 2019

@IamPete1 IamPete1 force-pushed the IamPete1:motor_sail branch from 9b4ca4d to de4a77d Jul 28, 2019

@IamPete1

This comment has been minimized.

Copy link
Contributor Author

commented Jul 28, 2019

This has now been tested, and seems to be working as expected. It also adds some sailboat navigation updates. A buffer angle has been added to so the heading controller is used for the 10deg bellow the no go angle, this results in much nice behaviour when the target heading is just on the no go angle, this happens on the last tack to a waypoint. A dead zone has been added for deciding which tack we are sailing on.

Bug fixed for acro tacking and auto mode tack time outs.

I think the only remaining thing is to work out how to best handle the control of sail and motor in manual mode, this uses a aux channel but there may be a better way. Also it would be nice to add a waypoint type to change the motoring state, ie use only the sail for waypoint A then use the motor for waypoint B

@IamPete1 IamPete1 force-pushed the IamPete1:motor_sail branch from de4a77d to 67958d6 Jul 28, 2019

@IamPete1 IamPete1 removed the WIP label Jul 29, 2019

APMrover2/sailboat.h Outdated Show resolved Hide resolved
APMrover2/sailboat.h Outdated Show resolved Hide resolved
@@ -68,14 +85,28 @@ class Sailboat
AP_Float sail_angle_ideal;
AP_Float sail_heel_angle_max;
AP_Float sail_no_go;
AP_Float max_cross_track;
AP_Float loit_radius;

This comment has been minimized.

Copy link
@rmackay9

rmackay9 Jul 30, 2019

Contributor

is the extra loiter radius really necessary? I guess the point is that a user might want to have different radius depending upon whether they are using a motor or are relying on just the sail.. it seems a bit of a luxury to me..

If you really think it's useful it's OK to leave it of course..

This comment has been minimized.

Copy link
@IamPete1

IamPete1 Jul 30, 2019

Author Contributor

Yeah that was the idea, we can use the normal loiter rad for the motor loiter and the new sailboat one for a sailing loiter. Not much point in using the motor in a loiter if you still have to have the big radius you need for the sailing loiter. Now we have a enable param I feel less bad about adding in more sailing stuff.

APMrover2/sailboat.cpp Outdated Show resolved Hide resolved
APMrover2/sailboat.cpp Outdated Show resolved Hide resolved

@IamPete1 IamPete1 force-pushed the IamPete1:motor_sail branch from 8a62a0d to a27b527 Jul 30, 2019

@IamPete1

This comment has been minimized.

Copy link
Contributor Author

commented Jul 30, 2019

I have removed a the complicated throttle stuff, this now has a aux option that can be used to control the only sail in only manual mode, if it is not set then it will be on the throttle as usual. This removes the need for the complicated arming checks. I have also added a fix for the PID issue that is on master following the recent changes.

@IamPete1 IamPete1 force-pushed the IamPete1:motor_sail branch 2 times, most recently from cdffbe6 to 29c88a4 Jul 30, 2019

@IamPete1

This comment has been minimized.

Copy link
Contributor Author

commented Aug 1, 2019

I have done a mavlink PR to add the waypoint type here: ArduPilot/mavlink#103

and a quick demo vid in SITL https://youtu.be/ho2hkkajMBo

This is now complete I think

@IamPete1 IamPete1 force-pushed the IamPete1:motor_sail branch from 750c192 to 022edc0 Aug 10, 2019

@IamPete1

This comment has been minimized.

Copy link
Contributor Author

commented Aug 10, 2019

@rmackay9 I have cherry picked over your fixes, Thanks!. I have had a go at refactoring calc_throttle, if your happy I can squash abit.

@IamPete1

This comment has been minimized.

Copy link
Contributor Author

commented Aug 12, 2019

save sailing speed control for later PR and only call throttle controller if we use the value

@IamPete1 IamPete1 force-pushed the IamPete1:motor_sail branch from 022edc0 to 7734069 Aug 13, 2019

if (throttle_state_t == Sailboat_Throttle::FORCE_MOTOR) {
rover.g2.motors.set_mainsail(100.0f);
return throttle_out;
return rover.control_mode->get_throttle_out(desired_speed);

This comment has been minimized.

Copy link
@rmackay9

rmackay9 Aug 20, 2019

Contributor

Hi, not a big fan of how the mode class's calc_throttle method is calling Sailboat's update_sail_control which is then calling back into Mode::get_throttle_out(). That's very confusing for the next developer. For me this is a clear sign that things are in the wrong place.

This comment has been minimized.

Copy link
@IamPete1

IamPete1 Aug 20, 2019

Author Contributor

I guess we could have a bool function to say if we should call get_throttle_out then call it from within calc throttle, like how the heading stuff works, there is on_indirect_route() to decide if we should call calc_heading()

@rmackay9

This comment has been minimized.

Copy link
Contributor

commented Aug 20, 2019

I've created a simplified version of the main motor assistance feature of this PR in this branch: https://github.com/rmackay9/rmackay9-ardupilot/commits/iampete1-motor-sail3

I'm sure it won't cause huge joy that it's so chopped down but I wanted to reduce it to a smaller set of changes so that it's easier to review, test and merge. Other changes can go in later as smaller PRs.

I haven't tested this in SITL yet but I hope to tomorrow

@rmackay9

This comment has been minimized.

Copy link
Contributor

commented Aug 21, 2019

@IamPete1, I've created this alternative PR: #12107 which has a reduced set of features from this PR but I think it's also merge-able as-is (assuming you don't find any bugs).

I hope you don't mind too much that I reworked it a bit. Most of the changes I made were non-functional changes (i.e. renamed variables and methods and moved things around) but I think I may have also fixed an issue with how tacking was enabled - for this I split the "throttle_assist" function into two functions one for assist-during-tacking the other assist-during-low-wind.

Anyway, please tell me if you see any issues with the new PR. We can work on getting the other bits like changes to the tacking logic in as a follow-up. txs!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.