-
Notifications
You must be signed in to change notification settings - Fork 28
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
External navigation interface #5
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is off to a good start. I looked mostly at the local updates for now and left a few thoughts.
px4_ros2_cpp/include/px4_ros2/navigation/experimental/local_navigation_interface.hpp
Outdated
Show resolved
Hide resolved
px4_ros2_cpp/include/px4_ros2/navigation/experimental/navigation_interface_codes.hpp
Outdated
Show resolved
Hide resolved
px4_ros2_cpp/src/navigation/experimental/local_navigation_interface.cpp
Outdated
Show resolved
Hide resolved
px4_ros2_cpp/src/navigation/experimental/local_navigation_interface.cpp
Outdated
Show resolved
Hide resolved
px4_ros2_cpp/src/navigation/experimental/local_navigation_interface.cpp
Outdated
Show resolved
Hide resolved
px4_ros2_cpp/include/px4_ros2/navigation/experimental/local_navigation_interface.hpp
Outdated
Show resolved
Hide resolved
px4_ros2_cpp/src/navigation/experimental/local_navigation_interface.cpp
Outdated
Show resolved
Hide resolved
examples/cpp/navigation/global_navigation/include/global_navigation.hpp
Outdated
Show resolved
Hide resolved
px4_ros2_cpp/include/px4_ros2/navigation/experimental/global_navigation_interface.hpp
Outdated
Show resolved
Hide resolved
px4_ros2_cpp/include/px4_ros2/navigation/experimental/global_navigation_interface.hpp
Outdated
Show resolved
Hide resolved
px4_ros2_cpp/include/px4_ros2/navigation/experimental/navigation_interface_base.hpp
Outdated
Show resolved
Hide resolved
px4_ros2_cpp/src/navigation/experimental/global_navigation_interface.cpp
Outdated
Show resolved
Hide resolved
px4_ros2_cpp/include/px4_ros2/navigation/experimental/navigation_interface_base.hpp
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great! Looking forward to some flight tests for validation 🛩️
examples/cpp/navigation/global_navigation/include/global_navigation.hpp
Outdated
Show resolved
Hide resolved
px4_ros2_cpp/include/px4_ros2/navigation/experimental/global_navigation_interface.hpp
Outdated
Show resolved
Hide resolved
px4_ros2_cpp/include/px4_ros2/navigation/experimental/global_navigation_interface.hpp
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work, I remarked a couple of smaller things.
examples/cpp/navigation/global_navigation/include/global_navigation.hpp
Outdated
Show resolved
Hide resolved
examples/cpp/navigation/global_navigation/include/global_navigation.hpp
Outdated
Show resolved
Hide resolved
px4_ros2_cpp/include/px4_ros2/navigation/experimental/navigation_interface_base.hpp
Outdated
Show resolved
Hide resolved
px4_ros2_cpp/include/px4_ros2/navigation/experimental/navigation_interface_base.hpp
Outdated
Show resolved
Hide resolved
px4_ros2_cpp/include/px4_ros2/navigation/experimental/global_navigation_interface.hpp
Outdated
Show resolved
Hide resolved
px4_ros2_cpp/include/px4_ros2/navigation/experimental/global_navigation_interface.hpp
Outdated
Show resolved
Hide resolved
px4_ros2_cpp/src/navigation/experimental/global_navigation_interface.cpp
Outdated
Show resolved
Hide resolved
px4_ros2_cpp/src/navigation/experimental/global_navigation_interface.cpp
Outdated
Show resolved
Hide resolved
px4_ros2_cpp/src/navigation/experimental/local_navigation_interface.cpp
Outdated
Show resolved
Hide resolved
This reverts commit 5083c22.
…lid estimate messages to errors
px4_ros2_cpp/src/navigation/experimental/local_navigation_interface.cpp
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The aux global position PR is merged, so this should be ready to go
px4_ros2_cpp/include/px4_ros2/navigation/experimental/local_navigation_interface.hpp
Outdated
Show resolved
Hide resolved
px4_ros2_cpp/include/px4_ros2/navigation/experimental/local_navigation_interface.hpp
Outdated
Show resolved
Hide resolved
px4_ros2_cpp/include/px4_ros2/navigation/experimental/local_navigation_interface.hpp
Outdated
Show resolved
Hide resolved
px4_ros2_cpp/include/px4_ros2/navigation/experimental/local_navigation_interface.hpp
Outdated
Show resolved
Hide resolved
px4_ros2_cpp/include/px4_ros2/navigation/experimental/local_navigation_interface.hpp
Outdated
Show resolved
Hide resolved
px4_ros2_cpp/include/px4_ros2/navigation/experimental/local_position_measurement_interface.hpp
Outdated
Show resolved
Hide resolved
px4_ros2_cpp/include/px4_ros2/navigation/experimental/local_position_measurement_interface.hpp
Outdated
Show resolved
Hide resolved
enum class PoseFrame | ||
{ | ||
Unknown, | ||
LocalNED, | ||
LocalFRD | ||
}; | ||
|
||
enum class VelocityFrame | ||
{ | ||
Unknown, | ||
LocalNED, | ||
LocalFRD, | ||
BodyFRD | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you either:
- move these into the class LocalPositionMeasurementInterface
- or make them more generally available by moving them into a common header
frame.hpp
? And then you could use a single enum.
Otherwise there will be a conflict if another component wants to define these as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll create a common header so that it's more widely available.
However, since they are slightly different, I'd be inclined to keep them as two separate enums.
px4_ros2_cpp/include/px4_ros2/navigation/experimental/local_position_measurement_interface.hpp
Outdated
Show resolved
Hide resolved
px4_ros2_cpp/include/px4_ros2/navigation/experimental/local_position_measurement_interface.hpp
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's get this in - I'm squashing the commits.
Great, thanks @bkueng! |
Contents:
Testing
vehicle_gps_position
plus noise, and checking aux global position was fused in the EKF.vehicle_local_position
plus noise, and checking external vision position was fused in the EKF.Usage
Build, source, run local interface example node which uses the interface to send a sample estimate every 1s:
Similarly for global interface example node: