Skip to content

Model based velocity fallback for DVL invalidity#531

Open
melihokur17 wants to merge 16 commits intomainfrom
melih/dvl_validity
Open

Model based velocity fallback for DVL invalidity#531
melihokur17 wants to merge 16 commits intomainfrom
melih/dvl_validity

Conversation

@melihokur17
Copy link
Contributor

No description provided.

@melihokur17 melihokur17 marked this pull request as ready for review February 3, 2026 20:52
Copy link
Collaborator

@eminmeydanoglu eminmeydanoglu left a comment

Choose a reason for hiding this comment

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

Everything is a model!
Leaving some notes, kudos

self.is_dvl_enabled = False

# Model-based velocity estimator variables
self.current_wrench = np.zeros(6)
Copy link
Collaborator

Choose a reason for hiding this comment

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

If controller goes ON -> OFF, the real wrench is zero, but current_wrench will be some stale nonzero numbers because the controller node doesn't publish wrench at all if controller is OFF. Not sure any possible issues from this can emerge?

Isn't it good practice to publish zero wrench if controller is OFF. After all the wrench is not nonexistent, it is really zero.

Comment on lines 261 to +270
if is_valid_msg.data:
rotated_vector = self.transform_vector(
[velocity_msg.linear.x, velocity_msg.linear.y, velocity_msg.linear.z]
)
velocity_msg.linear.x = rotated_vector[0]
velocity_msg.linear.y = rotated_vector[1]
velocity_msg.linear.z = rotated_vector[2]
self.last_valid_velocity = velocity_msg

self.update_twist_covariance(use_model_based=False)

Copy link
Collaborator

@eminmeydanoglu eminmeydanoglu Feb 3, 2026

Choose a reason for hiding this comment

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

DVL works: use it
Fails: model based estimate
okay, the model keeps its own self.estimated_velocity. but we never update it while DVL is working. So when DVL suddenly fails, will the model start from a stale value?

I guess yes. Look: only compute_model_based_velocity() writes self.estimated_velocity. And if odom is not received, it uses the old stale estimated_velocity as a starting point.

I guess the fix is simple: Just add the line
self.estimated_velocity[:3] = rotated_vector
under this if. wdy think?

Comment on lines +273 to +295
self.filter_cmd_vel()

if self.dynamic_model_available and dt > 0.001 and dt < 1.0:
model_vel = self.compute_model_based_velocity(dt)

velocity_msg.linear.x = model_vel[0]
velocity_msg.linear.y = model_vel[1]
velocity_msg.linear.z = model_vel[2]
velocity_msg.angular.x = model_vel[3]
velocity_msg.angular.y = model_vel[4]
velocity_msg.angular.z = model_vel[5]

self.update_twist_covariance(use_model_based=True)

else:
velocity_msg.linear.x = self.filtered_cmd_vel.linear.x
velocity_msg.linear.y = self.filtered_cmd_vel.linear.y
velocity_msg.linear.z = self.filtered_cmd_vel.linear.z
velocity_msg.angular.x = self.filtered_cmd_vel.angular.x
velocity_msg.angular.y = self.filtered_cmd_vel.angular.y
velocity_msg.angular.z = self.filtered_cmd_vel.angular.z

self.update_twist_covariance(use_model_based=True)
Copy link
Collaborator

Choose a reason for hiding this comment

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

the naming use_model_based confused me here (using it for cmd_vel's too). maybe is_measured is better?

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.

2 participants