Broadcom / AFBR-S50-API

API for the AFBR-S50 Time-Of-Flight Sensor Family.
https://www.broadcom.com/products/optical-sensors/time-of-flight-3d-sensors
BSD 3-Clause "New" or "Revised" License
22 stars 6 forks source link

API Explorer App Reference Manual corrections #34

Closed essej closed 3 weeks ago

essej commented 4 weeks ago

All items listed below refer to the HTML API App Reference for the Explorer app documentation, in all the measurement data commands section that starts here: https://broadcom.github.io/AFBR-S50-API/explorer_app_cmd_details.html#explorer_app_cmd_data

Error 1:

The Measurement Frame State Flags field of the data messages are notated as being HEX16 (2 bytes), but as of the current version of the Explorer API software, that field is actually HEX32 (4 bytes).

The source code defining that is here: https://github.com/Broadcom/AFBR-S50-API/blob/6fbd0e1053f602ffc5f8822c6a5a62f54b41b9d0/Sources/ExplorerApp/api/explorer_api_data.c#L79

Error 2: In the 3D data commands, there is also an Enabled Channel Mask HEX32 ( 4 bytes) sent just following the Enabled Pixel Mask field, but that is not shown in the reference html entries. The relevant code demonstrating that is here: https://github.com/Broadcom/AFBR-S50-API/blob/6fbd0e1053f602ffc5f8822c6a5a62f54b41b9d0/Sources/ExplorerApp/api/explorer_api_data.c#L92

Error 3:

The per-pixel data portions of the message only includes data for pixels included in the pixel mask, but it ALSO can include a reference pixel, if that pixel is active (which is almost always). As far as I can tell, a receiver should check the expected length of the message by counting the active pixels * the length of a pixel data (6 bytes), to determine whether the reference pixel is included. Then it will be placed after the other active pixel data for each of the status, range, and amplitude fields. This should be noted in the API reference, perhaps as extended description for the status, range, and amplitude fields. An example of where the code does this is here: https://github.com/Broadcom/AFBR-S50-API/blob/6fbd0e1053f602ffc5f8822c6a5a62f54b41b9d0/Sources/ExplorerApp/api/explorer_api_data.c#L159

Just thought it would save some others a bit of time and confusion to have the correct information in the API reference docs! I'm not sure where else to report these discrepancies, apologies if it is the wrong place.

c-berger commented 3 weeks ago

Hi there, thanks for those hints and the detailed explanation. Error 1 was actually already fixed by #32, just the HTML was not yet updated. Sorry for that inconvenience. I will fix Error 2 and 3 and let you know once its done.

c-berger commented 3 weeks ago

I have updated and fixed the documentation regarding your suggestions. https://broadcom.github.io/AFBR-S50-API/explorer_app_cmd_details.html#explorer_app_cmd_data In addition to the issues you mentions, also the ordering of data was wrong. The reference pixels values immediately follow the measuring pixels.

essej commented 3 weeks ago

That's great, thanks!