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.
The hwOS models support switching to a disabled gas mix. Therefore, the
disabled state is not always a good indication whether a gas mix is used
or not. Look for gas switches during the parsing step instead to keep
track of the actively used gas mixes.
Looking up the gasmix by oxygen and helium content is only needed for
the manual gas mixes. For gas switches to a fixed gas mix, the index is
stored directly in the data.
The hexdump only includes the command parameters, but not the main
command byte. Since there are many commands without parameters, that's
not very useful.
For dives in HP CCR mode, the oxygen and diluent tanks are stored at a
fixed index. This information is more reliable than using the tank name,
and also prevents the incorrect labeling of one of the other tanks as an
oxygen or diluent tank.
Firmware v84 introduced support for sidemount diving. Users can now
configure the two sidemount tanks as the source for the GTR (Gas Time
Remaining) estimations. We can take advantage of this feature to detect
the sidemount tanks. This is more reliable than using the tank name.
For open-circuit dives, the oxygen and diluent usage doesn't make any
sense at all. But when an open-circuit diver uses the letter 'D' to
indicate a tank for decompression use, it will get incorrectly labeled
as a diluent tank.
Fixed by restricting the oxygen/diluent usage to CCR dives only.
In commit 1c8cd096b57a876c4fb0afc5113aac05d75d924e the block size was
changed from 64 to 1024 bytes. For bluetooth classic communication, this
shouldn't matter, but for some reason it does cause the OSTC4 firmware
upgrade to fail. Maybe some buffering problem in the OSTC4 firmware or
bluetooth stack?
Change the block size back to 64 bytes.
