Garmin support: ignore FIT files that aren't dives

Linus Torvalds torvalds at linux-foundation.org
Tue Sep 4 16:38:03 PDT 2018


On Tue, Sep 4, 2018 at 3:00 PM Dirk Hohndel <dirk at hohndel.org> wrote:
>
> - in order to do that, my code makes the assumption that in the device_info
> message the device_index will always be included before the other elements

I really don't like that assumption, and it's wrong anyway. The device
index comes after the serial number in the stream, because the garmin
field packing seems to be big-to-small, probably due to structure
packing reasons.

And even if true, it just seems unnecessary. You could just check
whether the old data was zero or not, and not replace an already
filled-in field.

IOW, for the firmare fill, just do

DECLARE_FIELD(DEVICE_INFO, firmware, UINT16)
{
        if (!garmin->cache.firmware)
                garmin->cache.firmware = data;
}

and never mind the device_index at all - you'll just pick up the first
firmware version you see.

Oh, and no need to cast the 'data' to the right type. It will already
*be* of that type, the macros that create the field definition
functions will do that for you.

> Is there a hook that get's called when a message is completed, so
> when all fields are read?

Yes, this is what the

    struct record_data record_data;

is for: you can fill in fields in there, set the "pending" bit that
the collection of fields has been filled in, and then
flush_pending_record() will be called at the end of the full message.

This is needed for things like "gasmix" that aren't just a single
value, but a collection of values. And you *could* use it for this
too: first marshall the data into the record_data, and then in the
flush_pending_record() you can now look at the group of data together
(ie device_index and firmware fields)

However, for your case, I think the easier thing to do is to just do
that "just pick the first firmware version" and be done with it.

> - in the FIT file, the Descent Mk1 is identified as product 2859; I think this would
> make sense to use as model number (instead of 0)

I'll take a look.

              Linus


More information about the subsurface mailing list