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

Reorganize buoy_msgs #23

Merged
merged 5 commits into from
Aug 22, 2022
Merged

Reorganize buoy_msgs #23

merged 5 commits into from
Aug 22, 2022

Conversation

andermi
Copy link
Collaborator

@andermi andermi commented Aug 18, 2022

addresses #21 to support #20

Signed-off-by: Michael Anderson <[email protected]>
@andermi andermi linked an issue Aug 18, 2022 that may be closed by this pull request
Signed-off-by: Michael Anderson <[email protected]>
Signed-off-by: Michael Anderson <[email protected]>
Signed-off-by: Michael Anderson <[email protected]>
Copy link
Contributor

@quarkytale quarkytale left a comment

Choose a reason for hiding this comment

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

Works for me!

# find dependencies
find_package(ament_cmake REQUIRED)
find_package(rclcpp REQUIRED)
find_package(buoy_interfaces REQUIRED)
Copy link
Contributor

Choose a reason for hiding this comment

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

nit reorder


<exec_depend>buoy_interfaces</exec_depend>
<exec_depend>buoy_api_cpp</exec_depend>
<exec_depend>buoy_api_py</exec_depend>
Copy link
Contributor

Choose a reason for hiding this comment

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

nit reorder


<buildtool_depend>rosidl_default_generators</buildtool_depend>
<exec_depend>rosidl_default_runtime</exec_depend>

<depend>geometry_msgs</depend>
<depend>std_msgs</depend>
Copy link
Contributor

Choose a reason for hiding this comment

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

nit reorder

Copy link
Contributor

Choose a reason for hiding this comment

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

including L16 as well

# See the License for the specific language governing permissions and
# limitations under the License.

from ament_flake8.main import main_with_errors
Copy link
Contributor

Choose a reason for hiding this comment

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

Getting warnings here fir test_flake8.py and test_pep257.py

Warning: The (fspath: py.path.local) argument to Package is deprecated. Please use the (path: pathlib.Path) argument instead.
Warning: The (fspath: py.path.local) argument to Module is deprecated. Please use the (path: pathlib.Path) argument instead.
Warning: distutils Version classes are deprecated. Use packaging.version instead.

But I guess we can ignore them for now?

@@ -0,0 +1,4 @@
[develop]
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the empty resource file buoy_api_py/resource/buoy_api_py used for ?

<buildtool_depend>ament_cmake</buildtool_depend>

<depend>rclcpp</depend>
<depend>buoy_interfaces</depend>
Copy link
Contributor

Choose a reason for hiding this comment

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

nit reorder

@@ -0,0 +1,14 @@
# buoy_api_cpp

`buoy_api_cpp` provides API for MBARI Power Buoy.
Copy link
Contributor

Choose a reason for hiding this comment

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

Change to 'cpp API'?

std::vector<float> N_Spec; // RPM
std::vector<float> Torque_Spec; // N-m
std::vector<float> I_Spec; // Amps
JustInterp::LinearInterpolator<float> interp1d;
Copy link
Contributor

Choose a reason for hiding this comment

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

Saw this at a couple of places, is this variable name intentional?

@andermi andermi merged commit 29b23dd into main Aug 22, 2022
@andermi andermi deleted the andermi/reorg branch August 22, 2022 20:18
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.

Split buoy_msgs to pure messages and buoy API
2 participants