lgsvl / simulator

A ROS/ROS2 Multi-robot Simulator for Autonomous Vehicles
Other
2.29k stars 781 forks source link

rigidBody.velocity doesnt match simpleVelocity #347

Closed david-gwa closed 4 years ago

david-gwa commented 5 years ago

hey, there is a case when in visual, the agent is stopped, while npcC.GetVelocity() still return non-zero values. as following:

GetVelocity() rb.vel     (0.0, 0.0, 0.0)    simpleVel   (5.9, 0.3, 2.7)

how it happened ?

david-gwa commented 5 years ago

and this will make npc.state.velocity in python be zeros in some siutations

daviduhm commented 5 years ago

Which version of simulator are you using for this? Currently, our NPCs follows simple physics with non-kinematic rigidbody, meaning that Unity just teleports the NPC object rather than smoothly moving it by applying force. Because of this, the rigidbody of NPC should not have any non-zero velocity, and the simpleVel is supposed to be the correct one. Could you try with the latest version of simulator and let me know which specific case did you get such result?

david-gwa commented 5 years ago

hey, @daviduhm

I am using 2019.04 version. I will give an example:

in pythonAPI I want to set npc velocity to zero:

     s = npc.state
     s.velocity = lgsvl.Vector(0.0, 0.0, 0.0)
    npc.state = s 

in Api.Commands AgentSetState()

                    var rb = obj.GetComponent<Rigidbody>();
                    rb.velocity = velocity;
                    rb.angularVelocity = angular_velocity;

that's the problem, I expected the NPC be still, but here only rigidbody.velocity is set to 0.0, simpleVelocity is un controlled (?). then the output looks like:

      npc Sedan(Clone)be512087-67fb-4826-865c-8d9d5d6939b8 simple velocity: (-8.2, 0.0, 0.8) rb.velocity: (0.0, 0.0, 0.0)

and the npc is still driving in Python view

david-gwa commented 5 years ago

tracked simpleVelocity update in NPCControlComponent at line 723:

              simpleVelocity = (rb.position - lastRBPosition) / Time.deltaTime;

and in NPCControlComponent:NPCMove() at line 539:

                        rb.MovePosition(rb.position + transform.forward * currentSpeed * Time.deltaTime);

see here, rb.position is updated by simpleVelocity(non-zero), so which makes simpleVelocity keep non-zero, not as user expected

EricBoiseLGSVL commented 5 years ago

@david-gwa

Thanks for letting us know about this issue. Let me try to clarify what is going on.

When you are setting velocity using the API with AgentSetState, you are using NPC manual control. This is not a finished feature and was a temp fix so we could work on waypoint control. We plan on removing Manual and FixedSpeed control soon when we roll out the finished Waypoint control.

If you look at NPCController.cs in PhysicsUpdate() line 216, you can see that Manual and FixedSpeed Control never call SetTargetSpeed() that calculates simpleVelocity. Once we release waypoint control, this will be correctly calculated. We should have this released soon and will have much better features for direct control.

ntutangyun commented 4 years ago

Hi, I'd like to follow up on the update. Is it already implemented ? thanks.

EricBoiseLGSVL commented 4 years ago

@ntutangyun we have a task currently to fix a bug with world/local velocity. This should be released soon

ntutangyun commented 4 years ago

@EricBoiseLGSVL thank you for your reply.

EricBoiseLGSVL commented 4 years ago

@david-gwa @ntutangyun We are finishing up a refactor of NPC logic and should be released soon