Skip to content

Created Kalman Filter velocity publisher#398

Open
connor-wool wants to merge 3 commits into
masterfrom
connor-wool/localization/unit-tests
Open

Created Kalman Filter velocity publisher#398
connor-wool wants to merge 3 commits into
masterfrom
connor-wool/localization/unit-tests

Conversation

@connor-wool

@connor-wool connor-wool commented Feb 22, 2018

Copy link
Copy Markdown
Contributor

This code creates a publisher to expose the Kalman Filter's internal estimate of linear velocity.

The new data is published on the topic "/localization/debug/kf/linVel"

There are also two trivial spacing issues that were resolved in this commit.


This change is Reviewable

@skallaher

Copy link
Copy Markdown
Member

Reviewed 8 of 8 files at r1.
Review status: all files reviewed at latest revision, 7 unresolved discussions, some commit checks failed.


src/localization/CMakeLists.txt, line 22 at r1 (raw file):

#add the test directory
add_subdirectory(tests)

There is a missing EOF/endline character in this file.


src/localization/robosub_sensors.h, line 35 at r1 (raw file):

    void InputHydrophones(const geometry_msgs::Vector3Stamped::ConstPtr &msg);
    void InputOrientation(const geometry_msgs::QuaternionStamped::ConstPtr
        &msg);

The standard that Robosub has been working with for new lines is to align the new line with the portion of the previous line that it belongs in. For instance, here this change is incorrect, the original placement of &msg is desirable.


src/localization/tests/CMakeLists.txt, line 6 at r1 (raw file):

#add_dependencies(test_localization_system)
target_link_libraries(test_localization_system test_tools ${catkin_LIBRARIES} ${GTEST_LIBRARIES})

There is an extra newline here.


src/localization/tests/localizationSystem.test, line 2 at r1 (raw file):

<launch>
    <test test-name="test_localization_system" pkg="robosub" type="test_localization_system" time-limit="240.0 />

This is a rather generous timeout. 4 minutes is significant testing time. Is this necessary?


src/localization/tests/localizationSystem.test, line 3 at r1 (raw file):

<launch>
    <test test-name="test_localization_system" pkg="robosub" type="test_localization_system" time-limit="240.0 />
    

Trailing whitespace.


src/localization/tests/localizationSystem.test, line 6 at r1 (raw file):

    <rosparam command="load" file="$(find robosub)/param/cobalt.yaml" />

</launch>

Missing newline/EOF.


src/localization/tests/test_localization_system.cpp, line 21 at r1 (raw file):

{
    EXPECT_TRUE(true) << "This trivial test should never fail.";
}

This test is extraneous. Why is it here?


Comments from Reviewable

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.

2 participants