Mares Smart Dive Computer + Bluelink pro

Linus Torvalds torvalds at linux-foundation.org
Wed Sep 26 10:40:47 PDT 2018


On Wed, Sep 26, 2018 at 8:13 AM Dirk Hohndel <dirk at hohndel.org> wrote:
>
> Here's the APK
> http://subsurface-divelog.org/downloads/test/Subsurface-mobile-mares-test.apk

Never mind.

I added hacks to test the Mares case, and while the packet splitting
seems to work fine, the "read multiple packets to satisfy the whole
read" does *not* work.

The reason it doesn't work is annoying: we use the "did the user ask
for the number of bytes returned" to decide whether we should satisfy
the whole read or not.

And yes, Mares passes in "NULL" for the actual byte return, *BUT* the
qt-ble.c code never actually gets a NULL.

Why? Because the dc_iostream_read() will never pass the NULL down to
the low-level IO, it does

        size_t nbytes = 0;
        ...
        status = iostream->vtable->read (iostream, data, size, &nbytes);
        if (actual)
                *actual = nbytes;

so the qt-ble code will always see a non-NULL "actual" pointer
(because it will be that pointer to the 'nbytes' in the wrapper.

So all that code that says "if the caller doesn't want partial
packets" is basically dead code, and qt-ble.c will always juist return
one packet at a time.

Which is what all our *other* BLE users actually want. The Suunto EON
Steel downloader, for example, doesn't really know how many packets
will come, so waiting for some buffer full condition is absolutely not
correct, and it needs the "return partial" case.

I need to figure out some other way to determine that "should I wait
for more" condition.

I suspect I need to put the "repeat read" condition inside
libdivecomputer itself. Because right now the "user passed in a NULL
actual pointer" case is insane: dc_iostream_read() will happily return
a partial buffer with no way to see how much of it was filled.

Jef, what are the semantics for

        ret = dc_iostream_read(iostream, buf, sizeof(buf), NULL);

meant to be when there is only a partial read? Right now it returns
DC_STATUS_SUCCESS and garbage in the end of "buf". Which really does
seem wrong. The caller has no way to know that it got a just a partial
reply..

The serial driver rules seem to be "I will repeat until I get the
whole buffer", so it doesn't have that issue - it will always fill the
buffer completely, or return an error. But enforcing that kind of
behavior is crazy anyway: it makes the whole "actual" pointer totally
pointless.

And as mentioned, that "I need to always fill the whole buffer" isn't
actually possible or acceptable for other protocols like BLE. The
buffer size needs to be a whole packet, but not all packets are full
packets.

So I'd need some way to pass in "wait for the *whole* buffer to fill"
(which is what something like the Mares case needs - the downloader
knows exactly now many bytes to expect) vs "wait for *some* data to
arrive" (which is what something like the EON Steel requires, because
it does *not* know how many bytes it will actually get).

Note that the EON Steel case is really fundamental: even when it knows
how many final bytes to fill in (because it was reading a file), it
does *not* know how many bytes that is in a BLE packet, because the
BLE data is escaped. So it migth want 5 bytes of data in the end, but
with escaping that might be up to 10 bytes of actual data read over
BLE.

We could read things one byte at a time in the EON Steel back-end, but
that would be *horribly* expensive.

Using the "actual" pointer to see whether the user was ready to get a
partial packet or not seemed to be a really good idea., but the
dc_iostream_read() implementation sunk that idea.

                   Linus


More information about the subsurface mailing list