-
Notifications
You must be signed in to change notification settings - Fork 13.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
SENS: SF45: Improved Datahandling and fixes #23918
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -47,9 +47,8 @@ | |
#define SF45_MAX_PAYLOAD 256 | ||
#define SF45_SCALE_FACTOR 0.01f | ||
|
||
SF45LaserSerial::SF45LaserSerial(const char *port, uint8_t rotation) : | ||
SF45LaserSerial::SF45LaserSerial(const char *port) : | ||
ScheduledWorkItem(MODULE_NAME, px4::serial_port_to_wq(port)), | ||
_px4_rangefinder(0, rotation), | ||
_sample_perf(perf_alloc(PC_ELAPSED, MODULE_NAME": read")), | ||
_comms_errors(perf_alloc(PC_COUNT, MODULE_NAME": com_err")) | ||
{ | ||
|
@@ -69,15 +68,19 @@ SF45LaserSerial::SF45LaserSerial(const char *port, uint8_t rotation) : | |
} | ||
|
||
_num_retries = 2; | ||
_px4_rangefinder.set_device_id(device_id.devid); | ||
_px4_rangefinder.set_device_type(DRV_DIST_DEVTYPE_LIGHTWARE_LASER); | ||
|
||
// populate obstacle map members | ||
_obstacle_map_msg.frame = obstacle_distance_s::MAV_FRAME_BODY_FRD; | ||
_obstacle_map_msg.sensor_type = obstacle_distance_s::MAV_DISTANCE_SENSOR_LASER; | ||
_obstacle_map_msg.increment = 5; | ||
_obstacle_map_msg.angle_offset = 2.5; | ||
_obstacle_map_msg.min_distance = UINT16_MAX; | ||
_obstacle_map_msg.min_distance = 20; | ||
_obstacle_map_msg.max_distance = 5000; | ||
_obstacle_map_msg.angle_offset = 0; | ||
const uint32_t internal_bins = sizeof(_obstacle_map_msg.distances) / sizeof(_obstacle_map_msg.distances[0]); | ||
|
||
for (uint32_t i = 0 ; i < internal_bins; i++) { | ||
_obstacle_map_msg.distances[i] = UINT16_MAX; | ||
} | ||
|
||
} | ||
|
||
|
@@ -91,16 +94,12 @@ SF45LaserSerial::~SF45LaserSerial() | |
|
||
int SF45LaserSerial::init() | ||
{ | ||
|
||
param_get(param_find("SF45_UPDATE_CFG"), &_update_rate); | ||
param_get(param_find("SF45_ORIENT_CFG"), &_orient_cfg); | ||
param_get(param_find("SF45_YAW_CFG"), &_yaw_cfg); | ||
|
||
/* SF45/B (50M) */ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can remove the "/* SF45/B (50M) */" comment now that the rangefinder min/max values are no longer relevant |
||
_px4_rangefinder.set_min_distance(0.2f); | ||
_px4_rangefinder.set_max_distance(50.0f); | ||
_interval = 10000; | ||
|
||
start(); | ||
|
||
return PX4_OK; | ||
|
@@ -161,7 +160,6 @@ int SF45LaserSerial::collect() | |
float distance_m = -1.0f; | ||
|
||
/* read from the sensor (uart buffer) */ | ||
const hrt_abstime timestamp_sample = hrt_absolute_time(); | ||
|
||
|
||
|
||
|
@@ -214,7 +212,6 @@ int SF45LaserSerial::collect() | |
// Stream data from sensor | ||
|
||
} else { | ||
|
||
ret = ::read(_fd, &readbuf[0], 10); | ||
|
||
if (ret < 0) { | ||
|
@@ -262,7 +259,7 @@ int SF45LaserSerial::collect() | |
} | ||
|
||
PX4_DEBUG("val (float): %8.4f, raw: %s, valid: %s", (double)distance_m, _linebuf, ((_crc_valid) ? "OK" : "NO")); | ||
_px4_rangefinder.update(timestamp_sample, distance_m); | ||
|
||
|
||
perf_end(_sample_perf); | ||
|
||
|
@@ -687,8 +684,6 @@ void SF45LaserSerial::sf45_process_replies(float *distance_m) | |
{ | ||
switch (rx_field.msg_id) { | ||
case SF_DISTANCE_DATA_CM: { | ||
|
||
uint16_t obstacle_dist_cm = 0; | ||
const float raw_distance = (rx_field.data[0] << 0) | (rx_field.data[1] << 8); | ||
int16_t raw_yaw = ((rx_field.data[2] << 0) | (rx_field.data[3] << 8)); | ||
int16_t scaled_yaw = 0; | ||
|
@@ -700,18 +695,18 @@ void SF45LaserSerial::sf45_process_replies(float *distance_m) | |
} | ||
|
||
// The sensor is facing downward, so the sensor is flipped about it's x-axis -inverse of each yaw angle | ||
if (_orient_cfg == 1) { | ||
if (_orient_cfg == ROTATION_DOWNWARD_FACING) { | ||
raw_yaw = raw_yaw * -1; | ||
} | ||
|
||
// SF45/B product guide {Data output bit: 8 Description: "Yaw angle [1/100 deg] size: int16}" | ||
scaled_yaw = raw_yaw * SF45_SCALE_FACTOR; | ||
|
||
switch (_yaw_cfg) { | ||
case 0: | ||
case ROTATION_FORWARD_FACING: | ||
break; | ||
|
||
case 1: | ||
case ROTATION_BACKWARD_FACING: | ||
if (scaled_yaw > 180) { | ||
scaled_yaw = scaled_yaw - 180; | ||
|
||
|
@@ -721,37 +716,41 @@ void SF45LaserSerial::sf45_process_replies(float *distance_m) | |
|
||
break; | ||
|
||
case 2: | ||
case ROTATION_RIGHT_FACING: | ||
scaled_yaw = scaled_yaw + 90; // rotation facing right | ||
break; | ||
|
||
case 3: | ||
case ROTATION_LEFT_FACING: | ||
scaled_yaw = scaled_yaw - 90; // rotation facing left | ||
break; | ||
|
||
default: | ||
break; | ||
} | ||
|
||
// Convert to meters for rangefinder update | ||
// Convert to meters for the debug message | ||
*distance_m = raw_distance * SF45_SCALE_FACTOR; | ||
obstacle_dist_cm = (uint16_t)raw_distance; | ||
_current_bin_dist = ((uint16_t)raw_distance < _current_bin_dist) ? (uint16_t)raw_distance : _current_bin_dist; | ||
|
||
uint8_t current_bin = sf45_convert_angle(scaled_yaw); | ||
|
||
// If we have moved to a new bin | ||
|
||
if (current_bin != _previous_bin) { | ||
|
||
if (_current_bin_dist > _obstacle_map_msg.max_distance) { | ||
_current_bin_dist = _obstacle_map_msg.max_distance + 1; // As per ObstacleDistance.msg definition | ||
} | ||
|
||
// update the current bin to the distance sensor reading | ||
// readings in cm | ||
_obstacle_map_msg.distances[current_bin] = obstacle_dist_cm; | ||
_obstacle_map_msg.distances[current_bin] = _current_bin_dist; | ||
_obstacle_map_msg.timestamp = hrt_absolute_time(); | ||
|
||
_current_bin_dist = UINT16_MAX; | ||
_previous_bin = current_bin; | ||
} | ||
|
||
_previous_bin = current_bin; | ||
|
||
_obstacle_distance_pub.publish(_obstacle_map_msg); | ||
|
||
break; | ||
|
@@ -768,7 +767,7 @@ uint8_t SF45LaserSerial::sf45_convert_angle(const int16_t yaw) | |
|
||
uint8_t mapped_sector = 0; | ||
float adjusted_yaw = sf45_wrap_360(yaw - _obstacle_map_msg.angle_offset); | ||
mapped_sector = round(adjusted_yaw / _obstacle_map_msg.increment); | ||
mapped_sector = floor(adjusted_yaw / _obstacle_map_msg.increment); | ||
|
||
return mapped_sector; | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,6 @@ | ||
module_name: Lightware SF45 Rangefinder (serial) | ||
serial_config: | ||
- command: lightware_sf45_serial start -R 0 -d ${SERIAL_DEV} | ||
- command: lightware_sf45_serial start -d ${SERIAL_DEV} | ||
port_config_param: | ||
name: SENS_EN_SF45_CFG | ||
group: Sensors | ||
|
@@ -41,11 +41,11 @@ parameters: | |
The SF45 mounted facing upward or downward on the frame | ||
type: enum | ||
values: | ||
0: Rotation upward | ||
1: Rotation downward | ||
24: Rotation upward | ||
25: Rotation downward | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I never actually had the chance to test these on the bench. The non-standard Yaw cfg as well. |
||
reboot_required: true | ||
num_instances: 1 | ||
default: 0 | ||
default: 24 | ||
|
||
SF45_YAW_CFG: | ||
description: | ||
|
@@ -55,9 +55,9 @@ parameters: | |
type: enum | ||
values: | ||
0: Rotation forward | ||
1: Rotation backward | ||
2: Rotation right | ||
3: Rotation left | ||
4: Rotation backward | ||
6: Rotation left | ||
reboot_required: true | ||
num_instances: 1 | ||
default: 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be non-zero. There has to be some offset since the obstacle distance array element 0 starts and moves clockwise, however with an increment of 10 degrees there won't be coverage immediately to the left of the actual obstacle distance data
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a bit of a question on the definition of the angle offset.
as the original algorithm was the Vector Field Histogram, I assumed that the angle was also defined the same way, meaning the corresponding angle is in the centre of the bin.
Which would also make sense as on 1D distance sensor pointed to the front with a fov of zero, gets only mapped to the bin with index zero.
I mean, we can agree on a definition here, and then also properly document it in the message.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did a quick test where I set the fov of a 1D sensor pointing to the front to be 5 deg.
and then the adjacent bins are empty and only the bin with index 0 is populated. Which would support the statement above.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe a better test of this is a 1D sensor 20 degree field of view sensor forward facing. If 20 degrees is split down the middle would the bins occupied be 0, 1 and 71, 72? (assuming 5 degree increment).
I guess this could be changed and defined to include that wrapping effect: the angle offset is super non-intuitive to most users, specifically to those implementing new drivers where there is sparse info on the sign of the angle and what it means.
It doesn't make a huge difference on the rotating lidars since the degree resolution is small, but would make a difference on 1D lidars with a significant field of view.