ros-drivers / velodyne

ROS support for Velodyne 3D LIDARs
http://ros.org/wiki/velodyne
Other
657 stars 652 forks source link

raw_packet struct is semantically incorrect for some sensors #504

Open Mtestor opened 1 year ago

Mtestor commented 1 year ago

Is your feature request related to a problem? Please describe. In rawdata.h(pp) of velodyne_pointcloud we can see this code :

/** \brief Raw Velodyne packet.
 *
 *  revolution is described in the device manual as incrementing
 *    (mod 65536) for each physical turn of the device.  Our device
 *    seems to alternate between two different values every third
 *    packet.  One value increases, the other decreases.
 *
 *  \todo figure out if revolution is only present for one of the
 *  two types of status fields
 *
 *  status has either a temperature encoding or the microcode level
 */
struct raw_packet
{
  raw_block blocks[BLOCKS_PER_PACKET];
  uint16_t revolution;
  uint8_t status[PACKET_STATUS_SIZE];
};

Even though the size of the packet doesn't change (1206 bytes) and the implementation access the data like an array of bytes , this struct is incorrect for hdl64e-s3, VLP16, VLP32 and VLS128 sensors. It should have this format :

struct raw_packet
{
  raw_block blocks[BLOCKS_PER_PACKET];                // usually 1200 bytes
  uint32_t timestamp;                                 // 4 bytes of time since top of hours, might be synchronized with GNSS.
  uint8_t miscellaneous[MISCELLANEOUS_SIZE]           // miscellaneous bytes (content depend on model)
}

Describe the solution you'd like It will be good to have a comment informing the user of the difference in order to clear any confusion.

Describe alternatives you've considered We can also make different struct based on the model or we can make the name an alias for a continous array of the proper size. using raw_packet = uint8_t[sizeof(raw_block) + OTHERS_BYTES_SIZE]; // An alias representing data packets