Extension by wheel data

[ThomasNaderBMW]

Add a description

So here is a pull request that adds information about the wheels of a vehicle.
It should be a good basement, but for sure there is potential to discuss especially the reference systems.
Feel free to give input! :)

Some questions to ask:
What is this change?
It is about the perceivable information of the wheels.
What does it fix?
It can be used for sensor models or simply for visualization reasons.
Is this a bug fix or a feature? Does it break any existing functionality or force me to update to a new version?
Minor Change, it is a feature.
How has it been tested?
Not yet, but discussed with implementors of visualization interfaces.

Mention a member

Add @mentions of the person or team responsible for reviewing proposed changes.
@caspar-ai , @pmai

Check the checklist

• My code and comments follow the style guidelines and contributors guidelines of this project.
• I have performed a self-review of my own code.
• I have made corresponding changes to the documentation.
• My changes generate no new warnings.
• I have added tests that prove my fix is effective or that my feature works.
• New and existing unit tests / travis ci pass locally with my changes.

[kmeids]

Hello @ThomasNaderBMW ,
Considering that in some cases not all wheels have the same radius (e.g. agriculture tractor, front wheels are smaller) would it make sense to move optional double radius_wheel = 2; under your newly proposed message WheelData ?

[ThomasNaderBMW]

@kmeids : Thx for the feedback. I also was thinking about to move it. Now I did it!
What do you think about the number_wheels? It is also redundant if you simply count the fields of wheelData..

[kmeids]

@ThomasNaderBMW: I would recommend to keep the number of wheels in case some simulation environments can't/won't provide the detailed WheelData message. (or maybe in case osi implementation is taking place at an incremental level)

[kmeids]

@ThomasNaderBMW: Based on our discussion today here are some additional comments:

1. on rotation_rate, instead of Positive means, the vehicle moves forward (since wheels can move in one direction and the vehicle can be moving in the opposite direction, i.e. slipping) let's go with the counterclockwise and clockwise convention where counterclockwise is usually associated with +v values and clockwise with -v ones. (we still need to define from which perspective, the axle can be a good reference point)
2. I also suggest to extend the radius to include data about the rim radius (additional field), the initial radius definition can still remain the same.

[raue]

As mentioned in todays discussion. The purpose/use-case and of course the limitations of having the wheel information should be somehow precised. Otherwise people might look at it from different perspectives and expectations may arise, which could go beyond to what is intended - e.g. ABS/ESP folks.

If the purpose is more focused on visualization, then I would suggest taking the so-called wagon-wheel-effect into account. This nasty behaviour might ruin the whole visual appearance.

[pmai]

Looking good, with these proposed changes for cleanup + @kmeids and @raue comments.

[ThomasNaderBMW]

@kmeids and @pmai:
Please have a short look. I made most of the corrections, but I am still no sure if the description of the orientation rate is sufficient.. What do you think?

[caspar-ai]

Looks good, a few suggestions / questions but only around the commenting.

[caspar-ai]

To clarify here, do you mean that the z-value should take into account things like suspension damping? Rather than the position being fixed, e.g. while the car is at rest?

(is there a particular reason to highlight what the z-value is for? Is it to help explain the convention? If so, I think "x forward" from the previous line is sufficient, but don't feel strongly about it)

[ThomasNaderBMW]

Yes, I would see the damping representation in these coordinates (mostly z, but also x & y).
The line could be deleted from my side, it is just a hint, what you can do with the position.. @ccb

[caspar-ai]

This might be a very noddy question, so sorry in advance!

This is defined as (counter-)clockwise rotation around the wheel axle, is that always when considered from the y-positive direction of the vehicle's co-ordinate system? i.e. both wheels on an axle will typically have the same sign on their rotation rate.

Might be worth adding in a basic example to clarify. (or let me know if this is so commonly understood by people who know about these things it isn't worth clarifying!)

[kmeids]

@caspar-ai , yes that is true, this means that both wheels on an axle will typically have the same sign on their rotation rate.
We also discussed adding pictures and examples, it was left for @ThomasNaderBMW to do.

[ThomasNaderBMW]

Yes the "forward" concept was in my first proposal. But the definition is no very scientific, so we changed to the right hand rule.. But to be honest I still think that it is even more complicated now and not self-explaining..
@kmeids , what do you think. Shall we change to an easier convention?

[kmeids]

@ThomasNaderBMW, no need to change it, I think with a suitable pictogram there would be no problem understanding it.

[ThomasNaderBMW]

@kmeids ah ok. Could that be a task for the service provider? That would help a lot, especially to demonstrate it in an understandable way!

[ThomasNaderBMW]

I reworked the open actions with the last commit. I hope it is clearer now! :)

[paagkame]

I have created a new svg. Please have a look at it at open-simulation-interface\doc\images\OSI_RotationRate.svg
I am not sure though if the use of color is allowed and where to place the image best. Is there an easy way of checking the changes made in the documentation in the output? Please have a look if the changes are OK.

[kmeids]

@paagkame
I assume that by "F" you are referring to the moving direction (Forward).
This can be confused by F as in Force.
To evade any confusions, we should either state that F is referring to forward or just extend the arrow a bit linger and write "moving direction"

[kmeids]

@paagkame
Could you also remove the "V" after the "+" and "-" sign?

[paagkame]

SVG is updated with proposed changes.

[kmeids]

Output from CCB meeting - 11.11.2020

Actions:

1. @clemenshabedank will follow-up with @ThomasNaderBMW on the following comment : Rough Approximation: The z-value shows the damping.
2. Documentation review to be performed before the minor release.
3. Pull request to be merged by @ThomasNaderBMW after documentation review.

[paagkame]

This is a bit tricky for me to understand. Did I got it right:
There is a wheel frame that you use as a reference. The wheel frame used in the simulation may vary in size. You "measure" the distance of the wheel frame used in the simulation for one complete rotation and relate this to the reference wheel frame. That means the value describes how often the reference wheel frame rotates around its axle to equal the distance that the wheel frame used in the simulation would cover in one complete rotation.

[kmeids]

I am afraid I don't really get you point, let's discuss this offline.

[ThomasNaderBMW]

I simplified that, please have a check @paagkame and @kmeids

[kmeids]

Output from CCB meeting - 09.12.2020

Actions:

1. @paagkame could you please follow up on this? Extension by wheel data #430 (comment)

[kmeids]

Output from CCB meeting - 09.12.2020

Actions:

1. Pull request ready to be merged, by @ThomasNaderBMW.

[kmeids]

Hello @ThomasNaderBMW, is it possible to have this pull request merged before Christmas ? :D

[pmai]

Approved by CCB, ready for merge after build