The BLE changes in commit e83732e200620882b13804f1ca54c1ab90a38188 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.
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 <jef@libdivecomputer.org>
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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 new models appear to be compatible with the previous G2, but with
new model numbers and bluetooth names. The USB VID/PID for the G3 is
still unknown.
Reported-by: Greg McLaughlin <support@moremobilesoftware.com>
When no compass bearing is set on the dive computer, the stored value is
initialized to zero. Since this can also be a valid value, those zero
values are only ignored untill another non-zero value is present.
In later firmware versions, the value will get initialized to 0xFFFF
instead.
Returning disabled gas mixes to the application mainly results in lots
of unnecessary information. Therefore, remove all disabled gas mixes,
unless they are actively used. Many other dive computers do not even
include disabled gas mixes in the data.
Unlike previously assumed, the on/off state of each gas mix is actually
available in the PNF data format (opening record 4). For the older
Predator data format, this info isn't available and all gas mixes are
manually marked as enabled.
The IX3M 2 with the APOS5 firmware supports a new info record containing
the GPS coordinates. To be able to identify this new record type, the
previously reserved field at byte offset 52 is now used to store the
record type: zero for the existing sample record and one for the new
info record.
This also fixes the underlying problem of the zero timestamp in commit
3c50e91a1096332df66b2d33d64e5a8dc9136ab9, because the zero timestamp was
the result of incorrectly interpreting the first info record as a sample
record.
Previously the timestamp of the first sample was always a non-zero
value, but the IX3M 2 with the APOS5 firmware now appears to record a
timestamp of zero. This was incorrectly detected as a backwards time
jump because the time is also initialized to zero.
Sending the two byte command header and then waiting for the ack byte
before sending the remainder of the command payload causes problems for
some (but not all) users. Most likely the extra roundtrip time due to
waiting for the ack byte results in a delay that sometimes exceeds a
timeout in the dive computer while it's waiting for the payload data.
On the other hand, sending both the command header and the payload data
in one single packet is also not an option, because it causes problems
for some models, like the Mares Matrix. See commit
59bfb0f3189b14ae858650b851539d59e3fefe86 for details.
As an alternative solution, send the packet payload immediately after
the header, without waiting for the ack byte. This eliminates the extra
roundtrip time, while still sending out two separate bluetooth packets.
The functionality provided by the filter function is not only useful for
the built-in transports, but also for the applications. For example in
combination with a custom transport.
The USB I/O backend needs some additional information (e.g. interface
number and in/out endpoints) to setup the USB connection. This info is
currently maintained inside the descriptor filter function and gets
passed to the USB backend by means of the filter parameters.
This approach is not only unnecessary complex, but also makes it very
difficult to expose the filter function in the public api because the
data structures for those parameters are private.
Therefore, this data exchange is replaced with a direct mapping between
the USB VID/PID and the configuration info in the USB backend itself.
Passing the descriptor for which the filter function is being called is
a good practice and will also allow to implement some more specific
filtering in the future.
Returning disabled gas mixes to the application mainly results in lots
of unnecessary information. Therefore, remove all disabled gas mixes,
unless they are actively used. Many other dive computers do not even
include disabled gas mixes in the data.
The removal of the disabled gas mixes requires a two pass approach for
parsing the profile data. The first pass is only used to discover which
gas mixes are actively used during the dive. Next, all disabled and not
actively used gas mixes are removed from the list. Since removing one or
more gas mixes also invalidates the index of the remaining gas mixes,
the profile needs to be parsed again to report the new index in the gas
switch samples.
The original one based index is used as the stable gas mix id, used for
looking up the new gas mix index.