SICKAG / libsick_ldmrs

This package contains a library for communicating with the SICK LD-MRS line of laser scanners. For a ROS wrapper, see sick_ldmrs_laser.
Apache License 2.0
7 stars 18 forks source link

Potential bug for 8 layers LD-MRS vertical angle correction? #4

Closed nikodul closed 4 years ago

nikodul commented 4 years ago

Hi,

From src/devices/LuxBase.cpp, line 1881:

// Calculate the effective mirror tilt as a function of the beam tilt at 0 degree horizontally and the current horizontal angle
const double effMirrorTilt = m_beamTiltAngle / 2.0 * M_SQRT2 * std::sin(0.5 * hAngle - M_PI_4);

Can you explain where this formula comes from? It seems not documented in any document. We figured out when plotting the 3D point cloud of an 8 layers SICK sensor in RTMaps that the vertical spread of the points was not symmetrical against the lidar front axis. This seems to come from the here-above formula where the PI/4 factor introduces a strange offset to the hAngle to a sine: hAngle should be 0 at the center of the scan range (towards laser front), and in such case the vertical angle correction takes a sin(-PI/4) factor, which seems weird. Then at -45°, the correction factor si sin( -3PI/8) while at +45°, the correction factor is sin (-PI/8) -> correction factor is not the same on both sides of the scanning range. Is it normal?

We have 2 laser scanners mounted approximately at the same place on a vehicle, one pointing 45° left, the other pointing 45° right. Attached are two screenshots where both pointclouds from the two lasers are overlaid on the video and also displayed in a 3D view:

sick_v_angle_correction

Here, with your v angle correction formula: the vertical spread of the scans to not correspond on the right border of the left sensor and the left border of the right sensor.

sick_v_angle_correction_v2

When applying std::cos(0.5hAngle) or std::sin(0.5hAngle - M_PI_2) (which are opposite anyways), it looks better.

Any idea? Thanks a lot, Nicolas.

jspricke commented 4 years ago

The code is from the example driver provided by SICK. I didn't find a new version, maybe you got one with your scanner and could check the diff? Anyway, your change sounds reasonable, can you provide a pull request with the change?

nikodul commented 4 years ago

Thanks for the rapid feedback. We did not get any code from SICK, just figured out that this line introduced a strange behaviour in the vertical angle correction. However I cannot tell where this formula comes from and what is the right one in case this one is indeed incorrect... is it cos(0.5 hAngle) or sin(0.5 hAngle - PI/2) or anything else...?

jspricke commented 4 years ago

I don't have access to the hardware anymore, can you check with the manual or compare with the Sick software?

nikodul commented 4 years ago

The SICK software seems to reproduce the same weird behavior. We did not find any documentation in manuals where this formula is explained. We sent a support message to SICK with no answer so far (it was a few days ago, so maybe do they need some additional time).

mintar commented 4 years ago

You should check with SICK, but as far as I know the formula is correct. The SICK LD-MRS with 4 layers don't have any vertical angle correction, only the 8-layer LD-MRS has, because to get 8 layers, the mirror is mounted asymmetrically. In the 8-layer LD-MRS, the layers are bunched up closely together vertically at the left end of the scanning range and diverge towards the right, so the distance between the layers is greatest at the right side of the scan and smallest at the left side.

hAngle should be 0 at the center of the scan range (towards laser front), and in such case the vertical angle correction takes a sin(-PI/4) factor, which seems weird.

It isn't 0 at the center.

Then at -45°, the correction factor is sin( -3PI/8) while at +45°, the correction factor is sin (-PI/8) -> correction factor is not the same on both sides of the scanning range. Is it normal?

Yes, it isn't symmetrical. If you want symmetrical scans from your two LD-MRS, you have to flip one on its head.

mintar commented 4 years ago

You can also verify this behavior yourself: Slowly move an object downwards in the area where the two scanners overlap. If the formula is correct, it should first appear in the topmost layer of the left scanner and last in the bottommost layer of the left scanner.

nikodul commented 4 years ago

Thanks for the hint. That can make sense. We will double check.

nikodul commented 4 years ago

Hi again, we have investigated a little further. We are ok on the fact that the mirror tilt effet introduces an asymmetric correction depending on the horizontal angle, and the -PI/4 offset to the angle in the sine is correct. However the SICK viewer software does not seem to produce the right vertical spread when a sensor is mounted upside down:

Our assumption is that the code misses a factor -1 on the hAngle variable in the sin when the sensor is mounted upside down.

When mounted normally, and when looking at the incoming stream from the sensor, angle ticks go from +1000 to -1500 approximately, scanning right to left and layers index sequence are 0, 1, 2, 3, 0, 1, 2, 3.

When mounted upside down, angle ticks come from the sensor from -1000 to +1500, and layers index sequence are 3, 2, 1, 0, 3, 2, 1, 0.

Assuming that the sensor mechanics did not change (that the motor is not inverted, mirrors are not moved inside the sensor, etc.), the sensor is now scanning from left to right, and what was the upper layer (index 0) is not the lower layer (index 3) and so on. That means that the sensor firmware adds a -1 factor on the angle ticks, and changes layers indices so that 0 is still the upper layer and 3 the bottom layer. Changing the layers indices allows to keep the same code and beam tilt offsets. Changing the ticks sign preserves the orientation convention, but this change should be reflected in the vertical angle correction:

// Calculate the effective mirror tilt as a function of the beam tilt at 0 degree horizontally and the current horizontal angle
double hAngleFactor = 1.0;
if (m_upsideDownActive == true)
{
    hAngleFactor = -1.0;
}
const double effMirrorTilt = m_beamTiltAngle / 2.0 * M_SQRT2 * std::sin(0.5 * hAngleFactor * hAngle - M_PI_4);

Would that work? It seems to provide correct results.

Thank you for you help.

mintar commented 4 years ago

In my opinion, this "upside down" parameter shouldn't be used at all. When we talked to the SICK people 3 years ago, they mentioned that they were going to remove it completely.

The scanner should always output the same data, no matter how it is mounted. The point cloud is in the scanner frame, after all. Then you should use external methods to transform the data into whatever frame you want. For example, if you're using ROS, you would typically specify the mounting positions of your scanners using URDF and then transform between them using TF.

nikodul commented 4 years ago

That's indeed one way to solve the issue, depending on how much CPU you want to save. The interest of the upside down mode is the possibility indeed to spare 3D transforms on the point cloud even though it won't kill a core i7 from far. We don't use ROS but indeed there are ways to apply such 3D transforms as well in RTMaps. In the above-mentioned case, we apply some 3D transforms anyways to place all sensors data in the same reference si it would not change much indeed.

Thanks for your hints and rapid feedback on this topic. All the best,