search

RoboCup-SSL / ssl-vision

Shared Vision System For The RoboCup Small Size League
GNU General Public License v3.0
90 stars 111 forks source link

Assumed ball height not being sent in ball plugin #160

Closed rolfvdhulst closed 4 years ago

rolfvdhulst commented 4 years ago

The assumed height of the ball is important part of the calculation/ for end-users to be able to replicate calculations, but currently is not being sent with the protobuf message.

g3force commented 4 years ago

What do you need the value for?

There are different ball heights:

  1. The actual ball height if the ball is flying (not available in ssl-vision)
  2. The ball z-value based on the size of the ball (21.5mm)
  3. The ball z-value that is used for conversions from image to field coordinates (30mm, manually optimized many years ago, I guess because the ball is round?)

Without any additional documentation, I would expect the first one in the protobuf message.

rolfvdhulst commented 4 years ago

I actually mean option 3), the 30 mm here; in general the only reason I can see why you would send the camera parameters to the end user is so they can recompute the positions if they desire to do so (and e.g. find the 3d rays on which the ball should be). This is also the main purpose for sending the robot height. You might want to rename the field so that it does not get confused with option 1) or 2) though.

g3force commented 4 years ago

I rather see option 3 as part of the geometry, not the detection, because it is constant. My impression is though, that this is an internal value, optimized for the ssl-vision code and should not be exposed.

I can definitely see, why one need the robot height, but what exactly do you need this 30mm value for? :thinking:

rolfvdhulst commented 4 years ago

I don't mind nothing being sent, but why is the field then still a part of the protobuf message?

g3force commented 4 years ago

It was added ten years ago, but I do not think it was ever used. It could probably be deleted.

g3force commented 4 years ago

I'll close this issue and the related PR then.