If the allocator receives multiple identical rov_forces messages in a row, it currently only publishes thruster_forces only for the first of the identical messages.

See

I don't think this is the desired behaviour. It was probably done at some point for performance reasons, but I suggest removing this feature so that one message in always causes one message out.

Actually I think the reason was that out manipulator code was very CPU heavy, and we usually used the manipulators only when not using the thrusters, so this feature would avoid taking resources away from the manipulators.

It has some trailing 'l's on lines 183 and 184 which are not supported by Python 3.6.

On lines 4, 86, 93, and 97, it has Python 2 style print statements.

Should check which Python version is actually used by ROS, and fix the above if needed.

Change Otterholm to Manta. Or maybe even remove ROV specific names from the package altogether.

Using vortex_msgs/FloatArray64Stamped now, which is a message type that should be completely deleted. Should create something like vortex_msgs/ThrusterForces which should have a float64[] variable probably.

joy_node from joy sets a value between -1 and 1 for both LT and RT on the Xbox 360 Wired Controller. After the initial click the setpoint on LT and RT change from 0 to 1.

Reproduce error by running

• rosrun joy joy_node

joy is built upon xboxdrv, when xboxdrv is inspected directly LT and RT seem fine.

This should be done to all topics. The names should probably be grouped on the server, such as topics/imu.

The current code has the topic names hard-coded into each node, which makes it very easy to use the wrong name, and cumbersome to change names.

Even though some/most of the tests are failing, Travis displays the build as passing.
I suspect that the reason for this is that the errors from rostest is not being passed out to the Docker-container and thus the build never fails.

Errors from roslint exit with a non-zero code, so the problem definetly lies with rostest.

Here is a log with failing tests, but a passing build.

In particular, we should consider which, if any, need the vortex_ prefix.

nav_msgs/Odometry is used for state estimates, and vortex_msgs/FloatArray64Stamped is used for thruster forces. Both of these should be replaced with message types that make sense.

Resetting of setpoints is currently called in the control loop, at whatever frequency it runs at (10 Hz now). That means that when e.g. using depth hold, diving, then releasing the dive button, the new depth setpoint can be up to 100 ms old, which causes the ROV to move back toward the surface slightly. Should reset setpoints when receiving vortex_msgs/PropulsionCommand instead, probably.

I suggest replacing

  const uint8_t WRENCH_SURGE = 0;
  const uint8_t WRENCH_SWAY  = 1;
  const uint8_t WRENCH_HEAVE = 2;
  const uint8_t WRENCH_ROLL  = 3;
  const uint8_t WRENCH_PITCH = 4;
  const uint8_t WRENCH_YAW   = 5;

  const uint8_t POSITION_SURGE = 0;
  const uint8_t POSITION_SWAY  = 1;
  const uint8_t POSITION_HEAVE = 2;

  const uint8_t EULER_YAW   = 0;
  const uint8_t EULER_PITCH = 1;
  const uint8_t EULER_ROLL  = 2;

with three enum declarations inside the class.

The claw_stepper only has directional control at the moment. It could be useful to give a command to perform a given amount of rotations/steps for spinning valves and such. It could be especially useful when attempting to level the OBS when we know how many rotations are needed to level the platform.

Should be possible to ask the controller node "Hey go to this particular position and orientation".

The IMU board used on project Otterholm features a sensor fusion chip that improves the accuracy of the IMU.

The sensor fusion chip is sensor agnostic meaning that it can take data from most sensors as long as the sensor is I2C register readable. We can therefore feed the sensor fusion algorithm data from the pressure sensor to give us a more precise position estimate. kriswiner has a nice wiki on sensor fusion with the IMU board, this might be a good place to start implementing this.

Today we use a ROS action, which is probably not the most suited method. We only change gains from the human interface, so an action that can be called from other nodes is overkill (or at least not the right sort of kill).

Look at this crazy shit:

The controller node should not call resetSetpoint() in headingHold and depthHold when they are not active.

  if (activate_headinghold)
  {
    ...
  }
  else
  {
    resetSetpoints();
    tau.setZero();
  }

This is a problem when the different controllers is combined; like in depth and heading hold control mode.

The controllers should only be able to change the setpoint of the state they are regulating. Eg. depthHold should only be able to change the heave setpoint.

Fixing this fixes the ROV "floating" away from its depth hold setpoint when we are changing the yaw state with control command from joystick.

Everything fails because the tests haven't been maintained.

HoundCI is a continious integration service that will lint source code and put comments directly in Pull Requests.

Looks like a good service and seems easier to deal with than running roslint as a part of the test rig in Travis. The primary benefit with a service like this is that when review a PR we can focus on the things that actually matter, and let a bot handle the bikeshedding.

However, it does not support C++ out of the box, but this blog post walks through how one adds support. Seems fairly straight forward.

Here's an example on how the bot comments a PR (from Home-assistant):

Steppers should have individual speed control. The speed requirements for the claw stepper and the vertical stepper are different, so individual speed control would be nice.

There's probably a bunch of stupid mistakes that an IDE, such as PyCharm, can detect. Should fix those.

• joystick_interface
• light
• manipulator_interface
• sensor_interface
• thruster_interface

joystick_interface/launch/joystick_interface.launch launches three nodes:

• joy_node
• throttle
• joystick_interface

joy_node outputs on the topic joy. throttle reads that topic and outputs on a topic joy_throttle (which is default, it just uses <input_topic>_throttle unless otherwise specified). I've noticed that sometimes the nodes don't seem to connect, when inspecting the node graph with rqt_graph. Should check if this is still a problem, and if it is, fix it.

Prefixing all class member variables with something like m_ might make it easier to follow code.