Update libdivecomputer with changes in upstream as of 20240725 #66
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add the deco and rbt samples for the Oceanic Pro Plus 4 and Aeris Atmos AI 2.
Keeping the one based tank number interally allows to easily discard the pressure samples from invalid tanks. This avoid returning a huge tank number (e.g. 0xFFFFFFFF) when the internal tank number happens to be zero.
The ringbuffer boundary addresses (begin/end) should be ordered correctly, and the packet size should be smaller than the ringbuffer size, otherwise the code won't work as expected.
Some of the internal state is cached in local variables at the start of the function, and is updated only at the end of the function. But the contents of the packet buffer is never cached. As a result, the two can go out of sync when an error occurs and the function returns early. Trying to restore the original state is pointless if the corresponding data in the packet buffer is no longer available. Fixed by removing the local variables and always updating the internal state in-place to reflect the current state.
A dive computer typically writes its ringbuffer in the forward direction. Thus, when downloading the dives in reverse order (newest first), the ringbuffer needs to be read in the backward direction. However, some dive computers already re-arrange the data in the correct order, which means the data needs to be read in the opposite direction. To support this case without drastic changes in the logic, the rbstream implementation is extended to also support reading in the forward direction.
The ringbuffer calculations contained several flaws: The ringbuffer_normalize() function doesn't shift the result from the internal normalize call back to the range [begin,end-1]. The only reason why this bug didn't cause any issues yet, is because this function isn't used anywhere. The ringbuffer_distance() function can return a wrong result whenever the difference between the two values is an exact multiple of the ringbuffer size. For example: distance(x,x,n) == 0 or n (depending on the empty/full mode) distance(x,x+n,n) == 0 distance(x+n,x,n) == n So far this bug didn't cause any problems yet, because in practice this function is always used with values inside the safe range [begin,end-1]. For input values outside the safe range [begin,end-1], only larger values are accepted, while smaller values will trigger an assert. A zero-length ringbuffer (e.g. begin == end) results in a division by zero.
The two bytes in the command to read the logbook index are most likely not a single 16 bit number, but two 8 bit numbers specifiying a range. The same pattern can be found in the logbook pointers.
Swapping values 2 and 3 of the pointer mode has the advantage that only the mode with the largest value uses bytes as its unit, while all others use the page size as their unit.
Refactor the code to retrieve the profile pointers from the logbook entry into a single function that deals with all the quirks internally and simply returns a begin/end pair. To obtain the end pointer, the page size is now added to the value of the last pointer without taking into account the ringbuffer boundaries. The consequence are: - The boundary checks in the caller need to be relaxed to accept the end pointer as a valid value. - The check for the gap between the profiles can no longer compare the pointer values directly because the begin/end values are equivalent, but not equal.
Move the code to read and emit the device info into a separate function to reduce some code duplication. As a side effect, the check for devices without a logbook and profile ringbuffer needs to be performed earlier now, in order to prevent a division by zero in the progress events.
Move the code to read the ringbuffer pointers into a separate function that deals with all the quirks internally and simply returns two begin/end pairs. To obtain the logbook end pointer, the page size is now added to the value of the last pointer without taking into account the ringbuffer boundaries. Therefore the boundary checks in the caller need to be relaxed to accept the end pointer as a valid value. The profile begin/end pointers are not used anywhere and are only retrieved for diagnostics purposes.
To prepare the code to support reading the logbook ringbuffer in the forward direction, a new field is added to the layout data structure to indicate the direction of the ringbuffer, the workaround for handling an invalid pointer is extended to support both directions, and finally the correct parameters are passed to the rbstream reader.
Logging the ringbuffer pointers is very useful while investigating problems and adding support for new models.
For dive computers which are using an application specific proprietary pairing mechanism instead of the standard bluetooth pairing, we need to be able to exchange some additional information with the application. Therefore, 3 new BLE specific ioctl's are added: When a device has not been paired yet, libdivecomputer will request the PIN code from the application with DC_IOCTL_BLE_GET_PINCODE. Once the device has been paired successfully, the access code is passed back to the application with DC_IOCTL_BLE_SET_ACCESSCODE. On the next download, libdivecomputer will request this access code again from the application with DC_IOCTL_BLE_GET_ACCESSCODE. If no access code is available, the pairing procedure will start again by requesting the PIN.
The Aqualung i330R and Apeks DSX use a completely new communication protocol. The main (and most problematic) difference is the use of a proprietary bluetooth pairing mechanism instead of the standard bluetooth pairing. The data format remains more or less compatible with the previous models, with only the usual changes to the parser. The initial code and reverse engineering work was contributed by Janice, with further improvements and modifications for integration in libdivecomputer by Jef. Co-authored-by: Jef Driesen <[email protected]>
All Github actions using Node.js 16 are deprecated [1]. Update the following actions to a newer version using Node.js 20: - actions/checkout - actions/upload-artifact - microsoft/setup-msbuild [1] https://github.blog/changelog/2023-09-22-github-actions-transitioning-from-node-16-to-node-20/
The C library stdio.h header file already defines the EOF macro. Rename our macro to avoid the conflict.
The BLE changes in commit e83732e are causing major problems for some of the usb-serial enabled models, like the Puck Pro and Quad Air. These models appear to require a small delay of a few milliseconds between sending the two command bytes and the remainder of the command payload. I suspect the device is still busy processing those first two bytes, and thus not ready in time to receive the remaining data. Instead of manually adding a fixed delay, restore the previous behaviour and wait for the ack byte again. This has the advantage that the delay is automatically proportional to the response time of the dive computer. For the BLE communication nothing changes.
Since commit f5f855d, the tank number should remain one based instead of zero based.
For devices with a newer firmware version (2B) the profile ringbuffer ends already at 0xFE00, while for devices with an older firmware version (1D) it runs to the end of the memory.
Devices supporting BLE 4.2 (or higher) can use data packets with a payload size of up to 244 bytes. None of the current Mares models support this at the moment, but increasing the size on the receiving side already prepares the code for future models and remains fully backwards compatible.
The macros make the code a bit more compact, and adding support for new models becomes easier too.
The Mares Genius (and compatible models) uses a new command to download different types of objects, instead of manually reading and parsing the flash memory. This command also supports reading device properties like the serial number. This change is necessary because newer models like the Sirius no longer support reading directly from the flash memory.
The current implementation assumes a fixed order for the different record types to share some code with the older models. See commits [1] and [2] for more details. The main disadvantages of this approach are the extra complexity and the limited flexibility to adapt to future changes to the data format. To make the parsing code more future proof, split the code into separate functions for the Genius/Horizon and the older models. [1] Commit 8b06f2c (Horizon) [2] Commit feec939 (Genius)
The Mares Sirius uses the same communication protocol as the Genius, except for the fact that it uses a newer BLE version which supports larger data packets. The actual MTU is likely negotiated because we see different sizes like 244 and 182 bytes. We don't have access to this MTU size because not all BLE implementations can expose this information. Unfortunately not only the BLE packet size is variable, but also the size of the higher level data frames (used for downloading the content of the objects) is no longer fixed. The frame size appears to adapt to the BLE MTU size. This is most likely done to reduce the overhead and maximize the throughput. Although each frame ends with an OxEA byte, we can't rely on this knowledge to detect the end of the frame. The END byte is not escaped in the payload, and thus can also appear anywhere in the frame. As a workaround, we rely on the fact that each frame appears to be send as a single BLE packet. The only exception is the ACK byte, which gets send as a separate BLE packet if the command requires parameter data. Compatible models: Quad Ci, Puck 4 and Puck Air 2
The BLE (and USBHID) protocol stores the size of the payload as a single byte in the packet header. Hence the size of the payload is limited to a maximum of 255 bytes. For sending packets, there is an additional command byte present, which reduces the maximum payload size to 254 bytes. For receiving packets, there is no command byte present and thus the maximum payload size is 255 bytes. The Scubapro G3 sends BLE packets of 256 bytes, and that caused the download to fail because the receive buffer was one byte too small.
When using the dc_datetime_gmtime() function for a timestamp with a timezone offset, the timezone offset needs to be added to the timestamp prior to the call.
The Apeks DSX uses a completely different set of values for the dive mode, and the values for SIDEMOUNT (DSX) and FREEDIVE (others) happens to be identical. Since there are a few places where the code checks for FREEDIVE without taking into account the dive computer model, all DSX sidemount dives are accidentally treated as freedives. Fixed by taking into account the model number.
A few dive computers (e.g. Ratio and Divesoft) have an integrated GPS receiver and can report their location. Right now, the supported dive computers appear to report only a single location at the start of the dive, although their data format is capable of supporting multiple values. If necessary, multiple values can be reported in the future, or even a location sample can be added.
Both models are compatable with the previous models. The only surprise is that the Tern TX has the same model number as the Tern, while the Peregrine TX received a new model number. Reported-by: Sven Knoch <[email protected]> Reported-by: Greg McLaughlin <[email protected]>
Signed-off-by: Michael Keller <[email protected]>
|
I just discovered that this is somehow breaking Garmin support. |
|
My bad, it's just non-helpful UX and me leaving bluetooth checked when testing. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Tested with Subsurface, all seems to be working fine.