bit-bots / bitbots_motion

MIT License
7 stars 8 forks source link

multiple motion fixes and improvements #325

Closed SammyRamone closed 2 years ago

SammyRamone commented 2 years ago

Proposed changes

Necessary checks

timonegk commented 2 years ago

The change itself looks fine, but this pull request contains some stuff that has nothing to do with it. Should it be merged as well or was it an accident?

SammyRamone commented 2 years ago

The change itself looks fine, but this pull request contains some stuff that has nothing to do with it. Should it be merged as well or was it an accident?

Yeah the branch became my WIP branch and has a lot of stuff now :sweat_smile: I will clean it up and make it merge ready when I am finished with the current features

SammyRamone commented 2 years ago

currently the odom frame is not published. I don't really know why, since the broadcast is called

SammyRamone commented 2 years ago

Currently the odometry is partially broken on this branch, I want to fix this before merging

SammyRamone commented 2 years ago

additionally there is also currently the following warning, that should probably be resolved, but this maybe needs to be done in the bio_ik git [DynupNode-16] Warning: class_loader.impl: SEVERE WARNING!!! A namespace collision has occurred with plugin factory for class bio_ik_kinematics_plugin::BioIKKinematicsPlugin. New factory will OVERWRITE existing one. This situation occurs when libraries containing plugins are directly linked against an executable (the one running right now generating this message). Please separate plugins out into their own library or just don't link against the library and use either class_loader::ClassLoader/MultiLibraryClassLoader to open.

timonegk commented 2 years ago

My problem was on my side, I tested again and now it works, except of the warning that you mentioned. But I can look into that, I don't think it's related to this PR. For the odometry problem: That's what I mean, I think this can still be merged before because the current master is broken anyway, and by merging it, we can continue the work in other areas that don't need a working odometry.

SammyRamone commented 2 years ago

I already found out what the issue is. the time for the transform is not set. Just give me a few more minutes, maybe I can come up with a fix

SammyRamone commented 2 years ago

Odometry now works again. In my opinion it can be merged. Please have a look at the recent changes and merge if its okay for you @timonegk (also merge the related PRs in bringup and meta)

timonegk commented 2 years ago

Perfect, I didn't know you were so close to finding a fix