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

Update Follow me code to match upstream changes #1770

Merged
merged 6 commits into from
Jun 30, 2022

Conversation

junwoo091400
Copy link
Contributor

Context

Recently merged PR : PX4/PX4-Autopilot#19260, introduced whole new set of parameters to the follow-me (called FollowTarget in upstream) code. Therefore the aim is along with mavlink/MAVSDK-Proto#287, update the Follow Me code in MAVSDK

What changed

  1. Renaming of Parameters "NAV_" into "FLW_TGT_"
  2. Addition of two parameters : FLW_TGT_ALT_M and FLW_TGT_MAX_VEL

@junwoo091400
Copy link
Contributor Author

@JonasVautherin Can we have a generic function that handles these basic Parameter get behavior? It's a lot of copy & paste in both get & set in the implementation code 🤒

https://github.com/mavlink/MAVSDK/pull/1770/files#diff-0e2a47a501e78daff4d3fed04b889fb571366cacae01774b72da340d893fc9edR45-R53

@JonasVautherin
Copy link
Collaborator

Can we have a generic function that handles these basic Parameter get behavior?

I don't see a way 😅

@junwoo091400 junwoo091400 force-pushed the pr_followme_updates branch from 40714b4 to 92dce70 Compare May 16, 2022 17:23
@junwoo091400
Copy link
Contributor Author

Updated the implementation code & proto commit!

@junwoo091400 junwoo091400 force-pushed the pr_followme_updates branch from 92dce70 to dc656c3 Compare May 20, 2022 14:10
JonasVautherin
JonasVautherin previously approved these changes May 23, 2022
{
switch (follow_direction) {
case FollowMe::Config::FollowDirection::None:
Copy link
Contributor

@potaito potaito May 27, 2022

Choose a reason for hiding this comment

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

@JonasVautherin Is there a way to keep these for now, but mark them as deprecated?
It would be fairly straight forward to map the FollowMe::Config::FollowDirection settings to float angles.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There was a discussion about this and that was the first plan, but since MAVSDK v2 is being released, it was decided that breaking the compatability would be ok 🤔

mavlink/MAVSDK-Proto#287 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm up to speed now and agree with just having MAVSDK 2.0 break compatibility 😏

@junwoo091400
Copy link
Contributor Author

@JonasVautherin The PX4's side PR's SITL in local build with this latest changes is now passing 🎉 however, It seems like there's a merge conflict within MAVSDK due to the difference in the generated files:

src/mavsdk_server/src/generated/follow_me/follow_me.pb.cc
src/mavsdk_server/src/generated/follow_me/follow_me.pb.h 

Was the 'protoc_gen_mavsdk' updated again? Otherwise, I can't reproduce this difference via the tools/generate_from_protos.sh script 🤷

image

@JonasVautherin
Copy link
Collaborator

however, It seems like there's a merge conflict within MAVSDK due to the difference in the generated files:

@junwoo091400 yeah it's not your fault, you just got unlucky because I updated MAVSDK's dependencies (including protobuf) and it changed the generated code. I rebased main and generated the code again, I believe this should be fine.

On your end you'll have to make a clean build (because you need to rebuild the dependencies) 👍.

I hope this helps 😊

potaito added a commit to PX4/PX4-Autopilot that referenced this pull request May 30, 2022
Need to update MAVSDK with the following PR:
mavlink/MAVSDK#1770

SITL is failing now because the follow-me
perspectives are no longer defined the
same way in MAVSDK and in the flight task.
@junwoo091400
Copy link
Contributor Author

On your end you'll have to make a clean build (because you need to rebuild the dependencies) +1.

Wow this is remarkable. I did rm -rf build/default and then rebuilt & generated protos & fixed style, and I totally didn't expect the generated files to be different, but I will have to learn more about the build process to understand 👀 Thank you!

@JonasVautherin
Copy link
Collaborator

JonasVautherin commented May 31, 2022

and I totally didn't expect the generated files to be different

Are your generated files different? 🤔

The conflict for proto now is because the proto submodule got updated in main. When this PR is ready, we'll merge your mavsdk-proto PR, then this PR can point to it, and finally we can merge. But I guess this needs to wait on PX4, right?

@junwoo091400
Copy link
Contributor Author

Are your generated files different? thinking

It was different before I rebuilt (configured) via rm -rf build/default 😉 But now, it isn't! (which is why I was surprised)

For the PX4's side, we are set for this implementation. And the MAVSDK tests were fixed as well, so no need to wait for PX4 👌

@JonasVautherin JonasVautherin force-pushed the pr_followme_updates branch 2 times, most recently from 7acfcef to 5066086 Compare May 31, 2022 18:02
@JonasVautherin
Copy link
Collaborator

And the MAVSDK tests were fixed as well, so no need to wait for PX4 ok_hand

@junwoo091400 So you'd say it's ready to be merged?
@julianoes do you have an opinion? Should we go ahead and merge the mavsdk-proto and then this PR, or do you want to wait for PX4?

@junwoo091400
Copy link
Contributor Author

@junwoo091400 So you'd say it's ready to be merged?

Yes it's ready to be merged 👍

p.s. SITL tests failing, but that's expected since the container used in v1.11 and v1.12 doesn't have the current upstream PR's implementations in Follow Me (new Parameters): https://github.com/mavlink/MAVSDK/blob/main/.github/workflows/main.yml#L683

This FollowMe test failure will keep show up in other PRs in the MAVSDK, how should we deal with this?

@potaito
Copy link
Contributor

potaito commented Jun 1, 2022

This FollowMe test failure will keep show up in other PRs in the MAVSDK, how should we deal with this?

What do you mean @junwoo091400? It's not merged yet, so it should not be causing issues in any other MAVSDK PRs 🤔

@julianoes
Copy link
Collaborator

We need to merge mavlink/MAVSDK-Proto#287 and then update the submodule here, and then we can do the final check and merge here.

@julianoes
Copy link
Collaborator

@julianoes do you have an opinion? Should we go ahead and merge the mavsdk-proto and then this PR, or do you want to wait for PX4?

It needs to go into PX4 first, usually. Otherwise we end up with stuff that's not supported.

@JonasVautherin
Copy link
Collaborator

It needs to go into PX4 first, usually. Otherwise we end up with stuff that's not supported.

Agree. Let's wait until PX4/PX4-Autopilot#18026 is merged then 👍

@junwoo091400
Copy link
Contributor Author

What do you mean @junwoo091400? It's not merged yet, so it should not be causing issues in any other MAVSDK PRs thinking

Oh what I meant was that once this is merged, the follow me tests will fail since the MAVSDK always tests against the v1.11 and v1.12 PX4 SITL

@julianoes
Copy link
Collaborator

julianoes commented Jun 2, 2022

Oh what I meant was that once this is merged, the follow me tests will fail since the MAVSDK always tests against the v1.11 and v1.12 PX4 SITL

Right. So we need to ignore these tests for v1.11 and v1.12.

For now we can disable the integration test but we should come up with some sort of "test matrix", so a table which says which test works for which PX4 and ArduPilot version. I think this will be future work.

For now, what you could try is to check the version in the integration test using info->get_version() or something like that and then exit or do the test.

dagar pushed a commit to PX4/PX4-Autopilot that referenced this pull request Jun 15, 2022
Need to update MAVSDK with the following PR:
mavlink/MAVSDK#1770

SITL is failing now because the follow-me
perspectives are no longer defined the
same way in MAVSDK and in the flight task.
junwoo091400 added a commit to junwoo091400/PX4-Autopilot that referenced this pull request Jun 16, 2022
…er position and velocity filter

Follow me : tidied second order filter implementation

Added velocity filtered info to uORB follow target status message, and rebase to potaito/new-follow-me-task

FollowMe : Rebasing and missing definition fixes on target position second order filter

Follow Me : Remove Alpha filter delay compensation code, since second order filter is used for pose filtering now

Followme : Remove unused target pose estimation update function

Follow Target : Added Target orientation estimation logic

Follow Target : Replaced offset vector based setpoint to rate controlled orbital angle control

Follow Target : Bug fixes and first working version of rate based control logic, still buggy

Follow Target : Added target orientation, follow angle, orbit angle value into follow_target_status uORB message for debugging

Follow Target : Fix orbit angle step calculation typo bug

Follow Target : Few more fixes

Follow Target : Few fixes and follow angle value logging bug fix

Follow Target : Added lowpass alpha filter for yaw setpoint filtering

Follow Target : Remove unused filter delay compensation param

Follow Target : Add Yaw setpoint filter initialization logic and bufix for when unwrap had an input of NAN angle value

Follow Target : Add Yaw setpoint filtering enabler parameter

Follow Target : Change Target Velocity Deadzone to 1.0 m/s, to accomodate walking speed of 1.5 m/s

Follow Target : Add Orbit Tangential Velocity calculation for velocity setpoint and logging uORB topics

Follow target : Fix indentation in yaw setpoint filtering logic

Follow Target : Fix Follow Target Estimator timeout logic bug that was making the 2nd order pose filter reset to raw value every loop

Follow Target : Remove debug printf statement for target pose filter reset check

Follow Target : Add pose filter natural frequency parameter for filter testing

Follow Target : Make target following side param selectable and add target pose filter natural frequency param description

Follow Target : Add Terrain following altitude mode and make 2D altitude mode keep altitude relative to the home position, instead of raw local position origin

Follow Target : Log follow target estimator & status at full rate for filter characteristics test

Follow Target : Implementing RC control user input for Follow Target control

Follow Target : edit to conform to updated unwrap_pi function

Follow Target : Make follow angle, distance and height RC command input configurable

Follow Target : Make Follow Target not interruptable by moving joystick in Commander

Follow Target : reconfigure yaw, pitch and roll for better user experience in RC adjusting configurations, and add angular rate limit taking target distance into account

Follow Target : Change RC channels used for adjustments and re-order header file for clarity

Follow Target : Fix Parameters custom parent macro, since using DEFINE_PARAMETERS alone misses the parameter updates of the parent class, FlightTask

Follow Target : Fix Orbit Tangential speed actually being angular rate bug, which was causing a phenomenon of drnoe getting 'dragged' towards the target velocity direction

Follow Target : Final tidying and refactoring for master merge - step 1

Add more comments on header file

Follow Target : tidy, remove unnecessary debug uORB elements from follow_target_status message

Follow Target : Turn off Yaw filtering by default

Follow Target : Tidy maximum orbital velocity calculation

Follow Target : add yaw setpoint filter time constant parameter for testing and fix NAV_FT_HT title

Follow Target : Add RC adjustment window logic to prevent drone not catching up the change of follow target parameters

Follow Target : fixes

Follow Target: PR tidy few edits remove, and update comments

Follow Target : apply comments and reviews

Follow Target : Edit according to review comments part 2

Follow Target : Split RC adjustment code and other refactors

- Splitted the RC adjustment into follow angle, height and distance
- Added Parameter change detection to reset the follow properties
- Added comments and removed yaw setpoint filter enabler logic

Follow Target : Modify orbit angle error bufferzone bug that was causing excessive velocity setpoints when setpoint catched up with raw orbit setpoint

Follow Target : Remove buffer zone velocity ramp down logic and add acceleration and rate limited Trajectory generation library for orbit angle and velocity setpoint

Follow Target : Remove internally tracked data from local scope function's parameters to simplify code

Follow Target : Fix to track unwrapped orbit angle, with no wrapping

Follow Target : Apply user adjustment deadzone to reduce sensitivity

Follow Target : Apply suggestions from PR review round 2 by @potaito

Revert submodule update changes, fall back to potaito/new-followme-task

Follow Target : [Debug] Expose max vel and acceleration settings as parameters, instead of using Multicopter Position Controller
's settings

Follow Target : Use matrix::isEqualF() function to compare floats

Follow Target : Add Acceleration feedback enabler parameter and Velocity ramp in initial commit for overshoot phenomenon improvement

Follow Target : Implement Velocity feed forward limit and debug logging values

Follow Target : Apply Velocity Ramp-in for Acceleration as well & Apply it to total velocity setpoint and not just orbit tangential velocity component

Follow Target : Don't set Acceleration setpoint if not commanded

Follow Target : Use Jerk limited orbit angle control. Add orbit angle tracking related uORB values"

Follow Target : Add Orbit Angle Setpoint Rate Tracking filter, to take into consideration for calculating velocity setpoint for trajectory generator for orbit angle

Revert "Follow Target : Add Orbit Angle Setpoint Rate Tracking filter, to take into consideration for calculating velocity setpoint for trajectory generator for orbit angle"

This reverts commit a3f48ac.

Follow Target : Take Unfiltered target velocity into acount for target course calculation to fix overshoot orbit angle 180 deg flip problem

Follow Target : Remove Yaw Filter since it doesn't make a big difference in yaw jitterness

Follow Target : Remove velocity ramp in control & remove debug values from follow_target_status.msg

Follow Target : Tidy Follow Target Status message logging code

Follow Target : Remove jerk and acceleration settings from Follow Target orbit trajectory generation

Follow Target : Change PublicationMulti into Publication, since topics published are single instances

Follow Target : Edit comments to reflect changes in the final revision of Follow Target

Follow Target : Apply incorrectly merged conflicts during rebase & update Sticks function usage for getThrottled()

Follow Target : Apply final review comments before merge into Alessandro's PR

Apply further changes from the PR review, like units

Use RC Sticks' Expo() function for user adjustments to decrease sensitivity around the center (0 value)

Update Function styles to lowerCamelCase

And make functions const & return the params, rather than modifying them
internally via pointer / reference

Specify kFollowPerspective enum as uint8_t, so that it can't be set to negative value when converted from the parameter 'FLW_TGT_FP'

Fix bug in updateParams() to reset internally tracked params if they actually changed.

Remove unnecessary comments

Fix format of the Follow Target code

Fix Follow Perspective Param metadata

follow-me: use new trajectory_setpoint msg

Convert FollowPerspective enum into a Follow Angle float value

1. Increases flexibility in user's side, to set any arbitrary follow
angle [deg]
2. Removes the need to have a dedicated Enum, which can be a hassle to
make it match MAVSDK's side
3. A step in the direction of adding a proper Follow Mode (Perspective)
mode support, where we can support kite mode (drone behaves as if it is
hovering & getting dragged by the target with a leash) or a constant
orbit angle mode (Drone always on the East / North / etc. side, for
cinematic shots)

Continue fixing Follow Target MAVSDK code to match MAVSDK changes

- Support Follow Angle configuration instead of Follow Direction
- Change follow position tolerance logic to use the follow angle
*Still work in progress!

Update Follow Me MAVSDK Test Code to match MAVSDK v2 spec

- Add RC Adjustment Test case
- Change follow direction logic to follow angle based logic completely
- Cleanup on variable names and comment on code

follow-me: disable SITL test

Need to update MAVSDK with the following PR:
mavlink/MAVSDK#1770

SITL is failing now because the follow-me
perspectives are no longer defined the
same way in MAVSDK and in the flight task.

update copyright year

follow-me: mark uORB topics optional

Apply review comments

more copyright years

follow-me sitl test: simpler "state machine"

flight_mode_manager: exclude AutoFollowTarget and Orbit on flash contrained boards
dagar pushed a commit to PX4/PX4-Autopilot that referenced this pull request Jun 16, 2022
…er position and velocity filter

Follow me : tidied second order filter implementation

Added velocity filtered info to uORB follow target status message, and rebase to potaito/new-follow-me-task

FollowMe : Rebasing and missing definition fixes on target position second order filter

Follow Me : Remove Alpha filter delay compensation code, since second order filter is used for pose filtering now

Followme : Remove unused target pose estimation update function

Follow Target : Added Target orientation estimation logic

Follow Target : Replaced offset vector based setpoint to rate controlled orbital angle control

Follow Target : Bug fixes and first working version of rate based control logic, still buggy

Follow Target : Added target orientation, follow angle, orbit angle value into follow_target_status uORB message for debugging

Follow Target : Fix orbit angle step calculation typo bug

Follow Target : Few more fixes

Follow Target : Few fixes and follow angle value logging bug fix

Follow Target : Added lowpass alpha filter for yaw setpoint filtering

Follow Target : Remove unused filter delay compensation param

Follow Target : Add Yaw setpoint filter initialization logic and bufix for when unwrap had an input of NAN angle value

Follow Target : Add Yaw setpoint filtering enabler parameter

Follow Target : Change Target Velocity Deadzone to 1.0 m/s, to accomodate walking speed of 1.5 m/s

Follow Target : Add Orbit Tangential Velocity calculation for velocity setpoint and logging uORB topics

Follow target : Fix indentation in yaw setpoint filtering logic

Follow Target : Fix Follow Target Estimator timeout logic bug that was making the 2nd order pose filter reset to raw value every loop

Follow Target : Remove debug printf statement for target pose filter reset check

Follow Target : Add pose filter natural frequency parameter for filter testing

Follow Target : Make target following side param selectable and add target pose filter natural frequency param description

Follow Target : Add Terrain following altitude mode and make 2D altitude mode keep altitude relative to the home position, instead of raw local position origin

Follow Target : Log follow target estimator & status at full rate for filter characteristics test

Follow Target : Implementing RC control user input for Follow Target control

Follow Target : edit to conform to updated unwrap_pi function

Follow Target : Make follow angle, distance and height RC command input configurable

Follow Target : Make Follow Target not interruptable by moving joystick in Commander

Follow Target : reconfigure yaw, pitch and roll for better user experience in RC adjusting configurations, and add angular rate limit taking target distance into account

Follow Target : Change RC channels used for adjustments and re-order header file for clarity

Follow Target : Fix Parameters custom parent macro, since using DEFINE_PARAMETERS alone misses the parameter updates of the parent class, FlightTask

Follow Target : Fix Orbit Tangential speed actually being angular rate bug, which was causing a phenomenon of drnoe getting 'dragged' towards the target velocity direction

Follow Target : Final tidying and refactoring for master merge - step 1

Add more comments on header file

Follow Target : tidy, remove unnecessary debug uORB elements from follow_target_status message

Follow Target : Turn off Yaw filtering by default

Follow Target : Tidy maximum orbital velocity calculation

Follow Target : add yaw setpoint filter time constant parameter for testing and fix NAV_FT_HT title

Follow Target : Add RC adjustment window logic to prevent drone not catching up the change of follow target parameters

Follow Target : fixes

Follow Target: PR tidy few edits remove, and update comments

Follow Target : apply comments and reviews

Follow Target : Edit according to review comments part 2

Follow Target : Split RC adjustment code and other refactors

- Splitted the RC adjustment into follow angle, height and distance
- Added Parameter change detection to reset the follow properties
- Added comments and removed yaw setpoint filter enabler logic

Follow Target : Modify orbit angle error bufferzone bug that was causing excessive velocity setpoints when setpoint catched up with raw orbit setpoint

Follow Target : Remove buffer zone velocity ramp down logic and add acceleration and rate limited Trajectory generation library for orbit angle and velocity setpoint

Follow Target : Remove internally tracked data from local scope function's parameters to simplify code

Follow Target : Fix to track unwrapped orbit angle, with no wrapping

Follow Target : Apply user adjustment deadzone to reduce sensitivity

Follow Target : Apply suggestions from PR review round 2 by @potaito

Revert submodule update changes, fall back to potaito/new-followme-task

Follow Target : [Debug] Expose max vel and acceleration settings as parameters, instead of using Multicopter Position Controller
's settings

Follow Target : Use matrix::isEqualF() function to compare floats

Follow Target : Add Acceleration feedback enabler parameter and Velocity ramp in initial commit for overshoot phenomenon improvement

Follow Target : Implement Velocity feed forward limit and debug logging values

Follow Target : Apply Velocity Ramp-in for Acceleration as well & Apply it to total velocity setpoint and not just orbit tangential velocity component

Follow Target : Don't set Acceleration setpoint if not commanded

Follow Target : Use Jerk limited orbit angle control. Add orbit angle tracking related uORB values"

Follow Target : Add Orbit Angle Setpoint Rate Tracking filter, to take into consideration for calculating velocity setpoint for trajectory generator for orbit angle

Revert "Follow Target : Add Orbit Angle Setpoint Rate Tracking filter, to take into consideration for calculating velocity setpoint for trajectory generator for orbit angle"

This reverts commit a3f48ac.

Follow Target : Take Unfiltered target velocity into acount for target course calculation to fix overshoot orbit angle 180 deg flip problem

Follow Target : Remove Yaw Filter since it doesn't make a big difference in yaw jitterness

Follow Target : Remove velocity ramp in control & remove debug values from follow_target_status.msg

Follow Target : Tidy Follow Target Status message logging code

Follow Target : Remove jerk and acceleration settings from Follow Target orbit trajectory generation

Follow Target : Change PublicationMulti into Publication, since topics published are single instances

Follow Target : Edit comments to reflect changes in the final revision of Follow Target

Follow Target : Apply incorrectly merged conflicts during rebase & update Sticks function usage for getThrottled()

Follow Target : Apply final review comments before merge into Alessandro's PR

Apply further changes from the PR review, like units

Use RC Sticks' Expo() function for user adjustments to decrease sensitivity around the center (0 value)

Update Function styles to lowerCamelCase

And make functions const & return the params, rather than modifying them
internally via pointer / reference

Specify kFollowPerspective enum as uint8_t, so that it can't be set to negative value when converted from the parameter 'FLW_TGT_FP'

Fix bug in updateParams() to reset internally tracked params if they actually changed.

Remove unnecessary comments

Fix format of the Follow Target code

Fix Follow Perspective Param metadata

follow-me: use new trajectory_setpoint msg

Convert FollowPerspective enum into a Follow Angle float value

1. Increases flexibility in user's side, to set any arbitrary follow
angle [deg]
2. Removes the need to have a dedicated Enum, which can be a hassle to
make it match MAVSDK's side
3. A step in the direction of adding a proper Follow Mode (Perspective)
mode support, where we can support kite mode (drone behaves as if it is
hovering & getting dragged by the target with a leash) or a constant
orbit angle mode (Drone always on the East / North / etc. side, for
cinematic shots)

Continue fixing Follow Target MAVSDK code to match MAVSDK changes

- Support Follow Angle configuration instead of Follow Direction
- Change follow position tolerance logic to use the follow angle
*Still work in progress!

Update Follow Me MAVSDK Test Code to match MAVSDK v2 spec

- Add RC Adjustment Test case
- Change follow direction logic to follow angle based logic completely
- Cleanup on variable names and comment on code

follow-me: disable SITL test

Need to update MAVSDK with the following PR:
mavlink/MAVSDK#1770

SITL is failing now because the follow-me
perspectives are no longer defined the
same way in MAVSDK and in the flight task.

update copyright year

follow-me: mark uORB topics optional

Apply review comments

more copyright years

follow-me sitl test: simpler "state machine"

flight_mode_manager: exclude AutoFollowTarget and Orbit on flash contrained boards

Remove unnecessary follow_target_status message properties

- As it eats up FLASH and consumes uLog bandwidth
dagar pushed a commit to PX4/PX4-Autopilot that referenced this pull request Jun 16, 2022
…er position and velocity filter

Follow me : tidied second order filter implementation

Added velocity filtered info to uORB follow target status message, and rebase to potaito/new-follow-me-task

FollowMe : Rebasing and missing definition fixes on target position second order filter

Follow Me : Remove Alpha filter delay compensation code, since second order filter is used for pose filtering now

Followme : Remove unused target pose estimation update function

Follow Target : Added Target orientation estimation logic

Follow Target : Replaced offset vector based setpoint to rate controlled orbital angle control

Follow Target : Bug fixes and first working version of rate based control logic, still buggy

Follow Target : Added target orientation, follow angle, orbit angle value into follow_target_status uORB message for debugging

Follow Target : Fix orbit angle step calculation typo bug

Follow Target : Few more fixes

Follow Target : Few fixes and follow angle value logging bug fix

Follow Target : Added lowpass alpha filter for yaw setpoint filtering

Follow Target : Remove unused filter delay compensation param

Follow Target : Add Yaw setpoint filter initialization logic and bufix for when unwrap had an input of NAN angle value

Follow Target : Add Yaw setpoint filtering enabler parameter

Follow Target : Change Target Velocity Deadzone to 1.0 m/s, to accomodate walking speed of 1.5 m/s

Follow Target : Add Orbit Tangential Velocity calculation for velocity setpoint and logging uORB topics

Follow target : Fix indentation in yaw setpoint filtering logic

Follow Target : Fix Follow Target Estimator timeout logic bug that was making the 2nd order pose filter reset to raw value every loop

Follow Target : Remove debug printf statement for target pose filter reset check

Follow Target : Add pose filter natural frequency parameter for filter testing

Follow Target : Make target following side param selectable and add target pose filter natural frequency param description

Follow Target : Add Terrain following altitude mode and make 2D altitude mode keep altitude relative to the home position, instead of raw local position origin

Follow Target : Log follow target estimator & status at full rate for filter characteristics test

Follow Target : Implementing RC control user input for Follow Target control

Follow Target : edit to conform to updated unwrap_pi function

Follow Target : Make follow angle, distance and height RC command input configurable

Follow Target : Make Follow Target not interruptable by moving joystick in Commander

Follow Target : reconfigure yaw, pitch and roll for better user experience in RC adjusting configurations, and add angular rate limit taking target distance into account

Follow Target : Change RC channels used for adjustments and re-order header file for clarity

Follow Target : Fix Parameters custom parent macro, since using DEFINE_PARAMETERS alone misses the parameter updates of the parent class, FlightTask

Follow Target : Fix Orbit Tangential speed actually being angular rate bug, which was causing a phenomenon of drnoe getting 'dragged' towards the target velocity direction

Follow Target : Final tidying and refactoring for master merge - step 1

Add more comments on header file

Follow Target : tidy, remove unnecessary debug uORB elements from follow_target_status message

Follow Target : Turn off Yaw filtering by default

Follow Target : Tidy maximum orbital velocity calculation

Follow Target : add yaw setpoint filter time constant parameter for testing and fix NAV_FT_HT title

Follow Target : Add RC adjustment window logic to prevent drone not catching up the change of follow target parameters

Follow Target : fixes

Follow Target: PR tidy few edits remove, and update comments

Follow Target : apply comments and reviews

Follow Target : Edit according to review comments part 2

Follow Target : Split RC adjustment code and other refactors

- Splitted the RC adjustment into follow angle, height and distance
- Added Parameter change detection to reset the follow properties
- Added comments and removed yaw setpoint filter enabler logic

Follow Target : Modify orbit angle error bufferzone bug that was causing excessive velocity setpoints when setpoint catched up with raw orbit setpoint

Follow Target : Remove buffer zone velocity ramp down logic and add acceleration and rate limited Trajectory generation library for orbit angle and velocity setpoint

Follow Target : Remove internally tracked data from local scope function's parameters to simplify code

Follow Target : Fix to track unwrapped orbit angle, with no wrapping

Follow Target : Apply user adjustment deadzone to reduce sensitivity

Follow Target : Apply suggestions from PR review round 2 by @potaito

Revert submodule update changes, fall back to potaito/new-followme-task

Follow Target : [Debug] Expose max vel and acceleration settings as parameters, instead of using Multicopter Position Controller
's settings

Follow Target : Use matrix::isEqualF() function to compare floats

Follow Target : Add Acceleration feedback enabler parameter and Velocity ramp in initial commit for overshoot phenomenon improvement

Follow Target : Implement Velocity feed forward limit and debug logging values

Follow Target : Apply Velocity Ramp-in for Acceleration as well & Apply it to total velocity setpoint and not just orbit tangential velocity component

Follow Target : Don't set Acceleration setpoint if not commanded

Follow Target : Use Jerk limited orbit angle control. Add orbit angle tracking related uORB values"

Follow Target : Add Orbit Angle Setpoint Rate Tracking filter, to take into consideration for calculating velocity setpoint for trajectory generator for orbit angle

Revert "Follow Target : Add Orbit Angle Setpoint Rate Tracking filter, to take into consideration for calculating velocity setpoint for trajectory generator for orbit angle"

This reverts commit a3f48ac.

Follow Target : Take Unfiltered target velocity into acount for target course calculation to fix overshoot orbit angle 180 deg flip problem

Follow Target : Remove Yaw Filter since it doesn't make a big difference in yaw jitterness

Follow Target : Remove velocity ramp in control & remove debug values from follow_target_status.msg

Follow Target : Tidy Follow Target Status message logging code

Follow Target : Remove jerk and acceleration settings from Follow Target orbit trajectory generation

Follow Target : Change PublicationMulti into Publication, since topics published are single instances

Follow Target : Edit comments to reflect changes in the final revision of Follow Target

Follow Target : Apply incorrectly merged conflicts during rebase & update Sticks function usage for getThrottled()

Follow Target : Apply final review comments before merge into Alessandro's PR

Apply further changes from the PR review, like units

Use RC Sticks' Expo() function for user adjustments to decrease sensitivity around the center (0 value)

Update Function styles to lowerCamelCase

And make functions const & return the params, rather than modifying them
internally via pointer / reference

Specify kFollowPerspective enum as uint8_t, so that it can't be set to negative value when converted from the parameter 'FLW_TGT_FP'

Fix bug in updateParams() to reset internally tracked params if they actually changed.

Remove unnecessary comments

Fix format of the Follow Target code

Fix Follow Perspective Param metadata

follow-me: use new trajectory_setpoint msg

Convert FollowPerspective enum into a Follow Angle float value

1. Increases flexibility in user's side, to set any arbitrary follow
angle [deg]
2. Removes the need to have a dedicated Enum, which can be a hassle to
make it match MAVSDK's side
3. A step in the direction of adding a proper Follow Mode (Perspective)
mode support, where we can support kite mode (drone behaves as if it is
hovering & getting dragged by the target with a leash) or a constant
orbit angle mode (Drone always on the East / North / etc. side, for
cinematic shots)

Continue fixing Follow Target MAVSDK code to match MAVSDK changes

- Support Follow Angle configuration instead of Follow Direction
- Change follow position tolerance logic to use the follow angle
*Still work in progress!

Update Follow Me MAVSDK Test Code to match MAVSDK v2 spec

- Add RC Adjustment Test case
- Change follow direction logic to follow angle based logic completely
- Cleanup on variable names and comment on code

follow-me: disable SITL test

Need to update MAVSDK with the following PR:
mavlink/MAVSDK#1770

SITL is failing now because the follow-me
perspectives are no longer defined the
same way in MAVSDK and in the flight task.

update copyright year

follow-me: mark uORB topics optional

Apply review comments

more copyright years

follow-me sitl test: simpler "state machine"

flight_mode_manager: exclude AutoFollowTarget and Orbit on flash contrained boards

Remove unnecessary follow_target_status message properties

- As it eats up FLASH and consumes uLog bandwidth
@junwoo091400
Copy link
Contributor Author

Upstream PR has been merged. Good for merge to MAVSDK as well 👍

junwoo091400 added a commit to junwoo091400/PX4-Autopilot that referenced this pull request Jun 28, 2022
…er position and velocity filter

Follow me : tidied second order filter implementation

Added velocity filtered info to uORB follow target status message, and rebase to potaito/new-follow-me-task

FollowMe : Rebasing and missing definition fixes on target position second order filter

Follow Me : Remove Alpha filter delay compensation code, since second order filter is used for pose filtering now

Followme : Remove unused target pose estimation update function

Follow Target : Added Target orientation estimation logic

Follow Target : Replaced offset vector based setpoint to rate controlled orbital angle control

Follow Target : Bug fixes and first working version of rate based control logic, still buggy

Follow Target : Added target orientation, follow angle, orbit angle value into follow_target_status uORB message for debugging

Follow Target : Fix orbit angle step calculation typo bug

Follow Target : Few more fixes

Follow Target : Few fixes and follow angle value logging bug fix

Follow Target : Added lowpass alpha filter for yaw setpoint filtering

Follow Target : Remove unused filter delay compensation param

Follow Target : Add Yaw setpoint filter initialization logic and bufix for when unwrap had an input of NAN angle value

Follow Target : Add Yaw setpoint filtering enabler parameter

Follow Target : Change Target Velocity Deadzone to 1.0 m/s, to accomodate walking speed of 1.5 m/s

Follow Target : Add Orbit Tangential Velocity calculation for velocity setpoint and logging uORB topics

Follow target : Fix indentation in yaw setpoint filtering logic

Follow Target : Fix Follow Target Estimator timeout logic bug that was making the 2nd order pose filter reset to raw value every loop

Follow Target : Remove debug printf statement for target pose filter reset check

Follow Target : Add pose filter natural frequency parameter for filter testing

Follow Target : Make target following side param selectable and add target pose filter natural frequency param description

Follow Target : Add Terrain following altitude mode and make 2D altitude mode keep altitude relative to the home position, instead of raw local position origin

Follow Target : Log follow target estimator & status at full rate for filter characteristics test

Follow Target : Implementing RC control user input for Follow Target control

Follow Target : edit to conform to updated unwrap_pi function

Follow Target : Make follow angle, distance and height RC command input configurable

Follow Target : Make Follow Target not interruptable by moving joystick in Commander

Follow Target : reconfigure yaw, pitch and roll for better user experience in RC adjusting configurations, and add angular rate limit taking target distance into account

Follow Target : Change RC channels used for adjustments and re-order header file for clarity

Follow Target : Fix Parameters custom parent macro, since using DEFINE_PARAMETERS alone misses the parameter updates of the parent class, FlightTask

Follow Target : Fix Orbit Tangential speed actually being angular rate bug, which was causing a phenomenon of drnoe getting 'dragged' towards the target velocity direction

Follow Target : Final tidying and refactoring for master merge - step 1

Add more comments on header file

Follow Target : tidy, remove unnecessary debug uORB elements from follow_target_status message

Follow Target : Turn off Yaw filtering by default

Follow Target : Tidy maximum orbital velocity calculation

Follow Target : add yaw setpoint filter time constant parameter for testing and fix NAV_FT_HT title

Follow Target : Add RC adjustment window logic to prevent drone not catching up the change of follow target parameters

Follow Target : fixes

Follow Target: PR tidy few edits remove, and update comments

Follow Target : apply comments and reviews

Follow Target : Edit according to review comments part 2

Follow Target : Split RC adjustment code and other refactors

- Splitted the RC adjustment into follow angle, height and distance
- Added Parameter change detection to reset the follow properties
- Added comments and removed yaw setpoint filter enabler logic

Follow Target : Modify orbit angle error bufferzone bug that was causing excessive velocity setpoints when setpoint catched up with raw orbit setpoint

Follow Target : Remove buffer zone velocity ramp down logic and add acceleration and rate limited Trajectory generation library for orbit angle and velocity setpoint

Follow Target : Remove internally tracked data from local scope function's parameters to simplify code

Follow Target : Fix to track unwrapped orbit angle, with no wrapping

Follow Target : Apply user adjustment deadzone to reduce sensitivity

Follow Target : Apply suggestions from PR review round 2 by @potaito

Revert submodule update changes, fall back to potaito/new-followme-task

Follow Target : [Debug] Expose max vel and acceleration settings as parameters, instead of using Multicopter Position Controller
's settings

Follow Target : Use matrix::isEqualF() function to compare floats

Follow Target : Add Acceleration feedback enabler parameter and Velocity ramp in initial commit for overshoot phenomenon improvement

Follow Target : Implement Velocity feed forward limit and debug logging values

Follow Target : Apply Velocity Ramp-in for Acceleration as well & Apply it to total velocity setpoint and not just orbit tangential velocity component

Follow Target : Don't set Acceleration setpoint if not commanded

Follow Target : Use Jerk limited orbit angle control. Add orbit angle tracking related uORB values"

Follow Target : Add Orbit Angle Setpoint Rate Tracking filter, to take into consideration for calculating velocity setpoint for trajectory generator for orbit angle

Revert "Follow Target : Add Orbit Angle Setpoint Rate Tracking filter, to take into consideration for calculating velocity setpoint for trajectory generator for orbit angle"

This reverts commit a3f48ac.

Follow Target : Take Unfiltered target velocity into acount for target course calculation to fix overshoot orbit angle 180 deg flip problem

Follow Target : Remove Yaw Filter since it doesn't make a big difference in yaw jitterness

Follow Target : Remove velocity ramp in control & remove debug values from follow_target_status.msg

Follow Target : Tidy Follow Target Status message logging code

Follow Target : Remove jerk and acceleration settings from Follow Target orbit trajectory generation

Follow Target : Change PublicationMulti into Publication, since topics published are single instances

Follow Target : Edit comments to reflect changes in the final revision of Follow Target

Follow Target : Apply incorrectly merged conflicts during rebase & update Sticks function usage for getThrottled()

Follow Target : Apply final review comments before merge into Alessandro's PR

Apply further changes from the PR review, like units

Use RC Sticks' Expo() function for user adjustments to decrease sensitivity around the center (0 value)

Update Function styles to lowerCamelCase

And make functions const & return the params, rather than modifying them
internally via pointer / reference

Specify kFollowPerspective enum as uint8_t, so that it can't be set to negative value when converted from the parameter 'FLW_TGT_FP'

Fix bug in updateParams() to reset internally tracked params if they actually changed.

Remove unnecessary comments

Fix format of the Follow Target code

Fix Follow Perspective Param metadata

follow-me: use new trajectory_setpoint msg

Convert FollowPerspective enum into a Follow Angle float value

1. Increases flexibility in user's side, to set any arbitrary follow
angle [deg]
2. Removes the need to have a dedicated Enum, which can be a hassle to
make it match MAVSDK's side
3. A step in the direction of adding a proper Follow Mode (Perspective)
mode support, where we can support kite mode (drone behaves as if it is
hovering & getting dragged by the target with a leash) or a constant
orbit angle mode (Drone always on the East / North / etc. side, for
cinematic shots)

Continue fixing Follow Target MAVSDK code to match MAVSDK changes

- Support Follow Angle configuration instead of Follow Direction
- Change follow position tolerance logic to use the follow angle
*Still work in progress!

Update Follow Me MAVSDK Test Code to match MAVSDK v2 spec

- Add RC Adjustment Test case
- Change follow direction logic to follow angle based logic completely
- Cleanup on variable names and comment on code

follow-me: disable SITL test

Need to update MAVSDK with the following PR:
mavlink/MAVSDK#1770

SITL is failing now because the follow-me
perspectives are no longer defined the
same way in MAVSDK and in the flight task.

update copyright year

follow-me: mark uORB topics optional

Apply review comments

more copyright years

follow-me sitl test: simpler "state machine"

flight_mode_manager: exclude AutoFollowTarget and Orbit on flash contrained boards

Remove unnecessary follow_target_status message properties

- As it eats up FLASH and consumes uLog bandwidth
@junwoo091400
Copy link
Contributor Author

Regarding the 'proto' having conflicts, should I:

  1. Rebase the proto PR
  2. Reference that submodule commit?

@junwoo091400 junwoo091400 force-pushed the pr_followme_updates branch from 5066086 to abec43b Compare June 30, 2022 10:38
junwoo091400 and others added 3 commits June 30, 2022 12:46
Update follow_me_impl to take upstream changes into account

Update Follow-Me implementation code

- Take into account the follow_angle parameter
- Update parameter names to latest
- Update comments to handle depreciated follow_direction

Update follow_me_impl with unit updates etc

Follow Target new edits on impl and autogenerated libraries

- In progress, currently it still includes the legacy 'follow_direction'
support, which should be dropped

Drop follow_direction support and change param type mismatch bug

- Now only 'follow_angle' property allows setting the follow perspective
of the follow-me mode in PX4

Remove follow_direction support from follow_me

- Along with Proto submodule update

Update follow_me proto and impl / example files

- Rename min_height_m to follow_height_m, which follows the convention
in the improved follow me mode in PX4 upstream

Fix up Follow Me code implementations

- Residue of wrong commits I guess
@junwoo091400 junwoo091400 force-pushed the pr_followme_updates branch from abec43b to 707664b Compare June 30, 2022 10:48
@junwoo091400
Copy link
Contributor Author

proto merge conflict solved by updating the proto submodule commit in the first commit of this PR during rebase

@junwoo091400
Copy link
Contributor Author

Quick question, what is the point of having an integration test when PX4 has it's own MAVSDK SITL tests as well? Since I noticed that they are doing similar tests but are maintained in separate repositories 🤷

@junwoo091400
Copy link
Contributor Author

For now, what you could try is to check the version in the integration test using info->get_version() or something like that and then exit or do the test.

With the latest commit, this has been applied, and should skip the SITL tests for v1.11 and v1.12 for FollowMe (and hence CI should pass).

I went for this change as I wasn't sure rightaway how to actually make the matrix criteria work:

I thought of modifying the executable integration_tests_runner for each PX4 version SITL tests, but this seemed a bit inefficient considering for every SITL test (for v1.11, v1.12, v1.13), the integration_tests_runner would need to be re-built. So I wanted to ask if this is ok or not before making changes 👍 Any thoughts on this?

@potaito
Copy link
Contributor

potaito commented Jun 30, 2022

Quick question, what is the point of having an integration test when PX4 has it's own MAVSDK SITL tests as well? Since I noticed that they are doing similar tests but are maintained in separate repositories shrug

What do you mean exactly? I extened the existing PX4 SITL tests for follow-me

@junwoo091400
Copy link
Contributor Author

junwoo091400 commented Jun 30, 2022

Quick question, what is the point of having an integration test when PX4 has it's own MAVSDK SITL tests as well? Since I noticed that they are doing similar tests but are maintained in separate repositories shrug

What do you mean exactly? I extened the existing PX4 SITL tests for follow-me

I mean that the integration tests in mavsdk (Used in Mavsdk CI) is totally separate from mavsdk tests in PX4 (Used in PX4 CI). And they are not making use of each other: e.g. Mavsdk has Spiral trajectory follow test, while PX4 doe straight line follow test

@potaito
Copy link
Contributor

potaito commented Jun 30, 2022

Ah sorry, I thought we were in the PX4 repo 😅
Yes, this is a legitimate question. I would assume that MAVSDK and PX4 test different things, so the tests will never be fully identical.

@julianoes
Copy link
Collaborator

CI looks unrelated but is still confusing.

@julianoes julianoes merged commit 0f04519 into mavlink:main Jun 30, 2022
@junwoo091400
Copy link
Contributor Author

Ooh no! the CI didn't pass, I actually didn't test the integration test locally and expected to check CI's ouptut. So will need to verify that and apply the fix necessary as this change definitely didn't fix the CI pass 🙈

@julianoes
Copy link
Collaborator

I fixed the version check. It was a timing issue.

@julianoes
Copy link
Collaborator

Thanks @junwoo091400 for the contribution!

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