Update libdivecomputer from Upstream. #83
Merged
Conversation
This file contains hidden or 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
When an error occurs, simply re-sending the failed command doesn't always work. Sometimes we appear to receive some stale data from the previous attempt, despite the fact that the input queue is purged before retrying. To improve the chances of re-synchronizing on the start of the next packet, wait a bit longer before re-trying and also ignore all received bytes until an ACK byte is encountered. This isn't entirely reliable, because the payload data isn't escaped and can also contain ACK bytes, but it's better than nothing. Below is a real-life example of such a case. First, we hit a timeout: [8.122494] INFO: Write: size=2, data=E742 [8.122663] INFO: Write: size=8, data=708F020080000000 [11.130287] INFO: Read: size=0, data= [11.130308] ERROR: Failed to receive the answer. [in src/mares_iconhd.c:213 (mares_iconhd_packet)] Next, we retry a few times: [11.130321] INFO: Sleep: value=100 [11.230536] INFO: Purge: direction=1 [11.231117] INFO: Write: size=2, data=E742 [11.240494] INFO: Write: size=8, data=708F020080000000 [11.415317] INFO: Read: size=20, data=0000000035004F000C00000037004F0000000000 [11.415331] ERROR: Unexpected answer byte. [in src/mares_iconhd.c:219 (mares_iconhd_packet)] [11.415339] INFO: Sleep: value=100 [11.515889] INFO: Purge: direction=1 [11.517474] INFO: Write: size=2, data=E742 [11.517628] INFO: Write: size=8, data=708F020080000000 [11.517798] INFO: Read: size=13, data=1300000036004F000A00000037 [11.517803] ERROR: Unexpected answer byte. [in src/mares_iconhd.c:219 (mares_iconhd_packet)] That didn't really work, but the interesting part is that we did receive some data now. We also see the same data appear in the next retry: [11.517811] INFO: Sleep: value=100 [11.618398] INFO: Purge: direction=1 [11.618760] INFO: Write: size=2, data=E742 [11.619100] INFO: Write: size=8, data=708F020080000000 [11.619341] INFO: Read: size=1, data=AA [11.619349] INFO: Read: size=20, data=0000000035004F000C00000037004F0000000000 [11.619355] INFO: Read: size=20, data=37004F001E00000036004F001000000037004F00 [11.619360] INFO: Read: size=20, data=2C00000037004F001200000034004F0033000000 [11.619366] INFO: Read: size=20, data=37004F000000000037004F001E00000036004F00 [11.630104] INFO: Read: size=12, data=1300000036004F000A000000 [11.630122] INFO: Read: size=20, data=37004F002000000038004F001D00000038004F00 [11.630131] INFO: Read: size=17, data=2300000039004F000600000037004F00EA This time, the retry seems to have worked and we move on to the next request: [11.635160] INFO: Write: size=2, data=E742 [11.635359] INFO: Write: size=8, data=F08E020080000000 [11.687620] INFO: Read: size=1, data=AA [11.708476] INFO: Read: size=20, data=0000000035004F000C00000037004F0000000000 [11.719214] INFO: Read: size=20, data=37004F001E00000036004F001000000037004F00 [11.719231] INFO: Read: size=20, data=2C00000037004F001200000034004F0033000000 [11.719240] INFO: Read: size=20, data=37004F000000000037004F001E00000036004F00 [11.740569] INFO: Read: size=13, data=1300000036004F000A00000037 [11.740585] INFO: Read: size=20, data=004F002000000038004F001D00000038004F0023 [11.740592] INFO: Read: size=20, data=00000039004F000600000037004F00EAAA090010 This appears to have worked, but upon closer inspection you can see that the very last packet already contains the start of the response to the next request (AA090010). But we haven't send the next request yet! That means the response we received here is actually the response to one of the earlier (failed) request and this start of the next packet is our real response. So we're getting completely out of sync! Unfortunately the Mares protocol doesn't use checksums or sequence numbers, so we can't really detect this problem.
The Halcyon Symbios HUD and Handset support both BLE communication and USB mass storage mode. In both cases the dives have the same data format. Tested-by: Benjamin Kuch <[email protected]>
Mark some variable with the GCC 'unused' attribute to silence (harmless) compiler warnings (-Wunused-variable and -Wunused-but-set-variable). The variables are kept because they are valuable as documentation or for debugging purposes.
Use the common macro defined in the array.h header instead of duplicating the same macro. The C_ARRAY_ITEMSIZE macro is also moved to the common header.
The existing dc_descriptor_iterator function is missing a context parameter, and also doesn't follow the standard naming convention. Fixed by adding a new dc_descriptor_iterator_new function. For backwards compatibility, the old function remains present in the library and a macro is used to automatically redirect to the new function.
The descriptor functions do not modify the passed descriptor. This is now made explicit by marking the parameter as const.
The filter function should take into account the transports supported by the device descriptor. If a transport isn't supported, the filter function shouldn't indicate a match.
The rfcomm filter function is not only platform dependent (Linux only), but also difficult to use correctly. It should only be enabled for devices which support DC_TRANSPORT_SERIAL due to the serial port emulation layer of the bluetooth Serial Port Profile (SPP/rfcomm), and not for usb-serial communication. However, some dive computers models like the OSTC3 have both rfcomm and usb-serial enabled variants. Since the filter functions are shared by all models, the result isn't always reliable. Fixed by simply removing the rfcomm filter. For serial communication, every string will now be considered a match, which is the same result as with no filter function present.
Using a bluetooth enabled model (e.g the OSTC 2) as the default choice is very convenient when using only the --family command-line option to scan for bluetooth devices or establish a connection. That's because after commit ce2c3c6, the filter will no longer result in a match for the non-bluetooth enabled models.
The checksum can help to detect corrupt dives and/or bugs in the download code.
The model number isn't strictly needed anymore, because libdivecomputer uses the information in the version packet to the detect the dive computer model. However, since the internal lookup table is still based on the model number, having easy access to the model number in the debug logs is very convenient when adding support for new devices.
The Puck Lite re-uses the same model number as the Puck 4. Only the bluetooth device name appears to be different.
For the Sirius (and compatible models), the data/time information is only 4 bytes large instead of the default 10 bytes.
The fingerprint data is valuable information when trying to reproduce some issues reported by users.
Occasionally there are a few samples with a zero pressure value at the begin and/or the end of a dive. Those values result in an incorrect begin/end pressure. Fixed by ignoring the zero pressure values.
The lower nibble at offset 0x0C contains part of the dive time and only the higher nibble contains part of the temperature. Since this extra nibble ended up as the second decimal figure, the error is actually very small and nobody really noticed the mistake.
The Cressi Edy appears to set the disabled gas mixes to 0xFF. The check for the magic value 0xF0 doesn't catch this case and therefore the value 0xFF results in a gas mix with 165% O2, which is obviously invalid. Fixed by checking the high nibble only.
Replace hardcoded constants with a layout descriptor. This reduces the amount of model specific conditions, and makes it easier to add support for new models. The Cressi data format uses nibbles (4 bits) as the smallest unit of storage. Larger values are stored as one or more nibble, typically with a BCD (binary coded decimal) encoding applied. To decode such variable sized numbers, a generic function is added. The offsets in the layout descriptor are also specified in nibbles instead of bytes.
Reading the payload and the trailer byte separately simplifies the code in the caller because the destination buffer doesn't need space to store the extra trailer byte.
The Cressi Archimede protocol is very similar to the existing Edy protocol, with a few smaller differences: - No init3 command and baudrate change - Smaller packet size (32 instead of 128 bytes) - Modified read command with a 1 byte address The most curious change is the fact that the nibbles of all bytes are reversed (little endian). To simplify the processing of the data, the nibbles are swapped again immediately after downloading.
The Suunto d9/vyper2 protocol supports a command to synchronize the clock.
The Mares Sirius firmare upgrade to v1.6.16 also changed the header version to v2.0. Again, there appear to be no functional changes in the data format. See also commit 82712ce.
The ppO2 values are stored with a unit of 0.01 bar and not 0.1 bar.
The ID_GAS_TRANSMITTER record reports a tank pressure from a transmitter linked to a gas mix. Use this information to link the tank to the corresponding gas mix.
Log format version 1.6 introduces some new record types. The ID_TANK_TRANSMITTER record reports a tank pressure from a transmitter not linked to a gas mix. Unlike the previous ID_GAS_TRANSMITTER record, the tank id is no longer the transmitter serial number, but a transmitter index (0-3). To avoid conflicts and being able to distinguish the two types, one of the higher bit is set internally.
Halcyon changed the bluetooth device name to the full serial number without a prefix. The serial number uses the format YYMMPCNNNN (e.g. 2412070123) with: - YY = Production year - MM = Production month - PC = Product code (01 = HUD, 07 = handset) - NNNN = Continuous number The existing filter with the prefix is kept for backwards compatibility.
The USB matching functions have been moved to the top so that all the string based matching functions are grouped together.
The prefix based matching functions have been renamed so that their function names share the same prefix.
The other prefix matching function already use a case-insensitive comparision. There is no reason for a different behaviour.
The Cressi Nepto with firmware v209 uses format version 4. Since the exact range isn't known, assume all firmware versions between 200 and 299 use the same format.
The model number in the bluetooth device name appears to be encoded as a hexadecimal number instead of a decimal number (e.g. a_xxxx vs 10_xxxx for the Nepto).
The decompression stop depth values are stored with a unit of centimeters and not 1 meter.
The hardware info contains a 32 bit model number that can be used to distinguish the different models. There is also a serial number prefix (at offset 8) which contains a single letter that serves the same purpose: A = Action B = Screen E = Tablet
Errors are reported by the dive computer with a NAK packet containing an error code. Detect and handle such packets and log the error code.
Ignoring all received bytes until an START bytes is encountered makes the re-synchronization on the start of the packet a bit more robust.
The Seac Tablet uses almost the same protocol as the Seac Screen and Action, but with a few small differences: - A slightly different set of commands, but with exactly the same functionality. - The new variant of the READ command does no longer pad the response packet to 2048 bytes. - The profile area starts at address 0x0A0000 instead of 0x010000. The Seac Tablet also support tank pressure transmitters.
There was a problem hiding this comment.
Pull Request Overview
This PR updates libdivecomputer with multiple upstream changes including new support for devices, improvements to function signatures, and documentation updates. Key changes include:
- Replacing the old dc_descriptor_iterator function with dc_descriptor_iterator_new and marking related parameters as const.
- Adding support for the Halcyon Symbios and updating hardware constants.
- Updating documentation and build files to reference the new function name and added files.
Reviewed Changes
Copilot reviewed 74 out of 74 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| include/libdivecomputer/descriptor.h | Updated iterator function signature and maintained backward compatibility via a macro. |
| include/libdivecomputer/common.h | Added new device family constant for the Halcyon Symbios. |
| examples/dctool_list.c | Adjusted the iterator function call to use the new API. |
| examples/common.c | Updated hardware constant for "ostc3". |
| doc/man/* | Updated man pages to reference dc_descriptor_iterator_new consistently. |
| contrib/msvc & contrib/android | Added new source and header files for Halcyon Symbios support. |
Comments suppressed due to low confidence (1)
examples/common.c:82
- The change of the hex constant for 'ostc3' from 0x0A to 0x11 should be verified against hardware specifications to ensure it is intentional.
{"ostc3", DC_FAMILY_HW_OSTC3, 0x11},
| dc_descriptor_iterator (dc_iterator_t **iterator); | ||
| dc_descriptor_iterator_new (dc_iterator_t **iterator, dc_context_t *context); | ||
|
|
||
| /* For backwards compatibility */ |
There was a problem hiding this comment.
The backward compatibility macro may obscure the behavior of the updated function. Consider adding a comment to clarify its intended use or reevaluate its necessity.
Suggested change
| /* For backwards compatibility */ | |
| /** | |
| * Backward compatibility macro for creating an iterator to enumerate | |
| * supported dive computers. This macro is a shorthand for calling | |
| * dc_descriptor_iterator_new with the second parameter set to NULL. | |
| * | |
| * @param[out] iterator A location to store the iterator. | |
| * @note This macro is intended for legacy code and assumes that the | |
| * context parameter is not required. For new code, it is | |
| * recommended to use dc_descriptor_iterator_new directly to | |
| * explicitly specify the context parameter. | |
| */ |
Copilot uses AI. Check for mistakes.
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.
Included in this pull request:
Tested with: