search

fsstudio-team / ZeroSimROSUnity

Robotic simulation in Unity with ROS integration.
https://roboticsimulationservices.com/
MIT License
174 stars 21 forks source link

Fix incorrect coordinate system in imu publisher #10

Open rendellc opened 3 years ago

rendellc commented 3 years ago

Addresses the bug discussed in #9.

Changes the coordinate transformation done for one of the CoordinateSystemEnum. Not sure if the changes are inline with the intended behaviour, but this PR serves as a place to discuss it.

micahpearlman commented 3 years ago

@rendellc are you using and testing that particular .RightHanded_XBackward_YLeft_ZDown coordinate system?

It seems like every IMU we run across has a different coordinate system and it is no surprise this is a bit of a mess. I know internally with different projects we end up just hacking at this just to get it to work for a particular IMU that a client uses.

rendellc commented 3 years ago

Hey, thanks for the clarification. I was a bit confused about where the different coordinate systems came from, so that makes sense. As mentioned in #9, we do not use RightHanded_XBackward_YLeft_ZUp nor the proposed bug fix RightHanded_XBackward_YLeft_ZDown, so we are not able to test this properly and compare it to a real IMU. Do you know if there is a particular IMU that this coordinate system is supposed to represent? If it is hacked together for a very specific use-case then it may be risky to change its behaviour. Maybe it's more appropriate to add a comment with an explanation?

micahpearlman commented 3 years ago

@rendellc I'm thinking of deleting or commenting out all the untested coordinate systems as I'm not clear on the state of things without creating some sort of test harness -- which is a TODO. Which coordinate system are you using?

rendellc commented 3 years ago

We are using RightHanded_XRight_YDown_ZForward. An option is to let the users specify a (possibly improper) rotation matrix to convert from unity coordinates to IMU coordinates. This would allow conversion from Unitys left-handed coordinate system to any left- or right-handed coordinate system, and we could remove the enums. Not sure how much work this is to implement, and a disadvantage is that it is slightly less intuitive than the current setup.

micahpearlman commented 3 years ago

@rendellc that is a very good idea!