Garmin: expose FIT product models

[mikeller]

Register each supported FIT product ID as an individual descriptor, and select MTP from the existing model capability table rather than a Mk2-specific comparison. Add a parser device-info query so FIT imports can select the detected descriptor before parsing.

[Copilot]

🟡 Changes recommended

Resolve model-list consistency and initialize hw_id in the device-info API.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds individual Garmin FIT product descriptors and parser device-info lookup.

Changes:

• Registers Garmin product descriptors.
• Uses the shared model table for MTP selection.
• Exports dc_parser_get_device_info().

File summaries

File	Summary
test/fixtures/manifest.txt	Updates the Garmin fixture descriptor name.
src/parser.c	Implements parser device-info lookup.
src/libdivecomputer.symbols	Exports the new API symbol.
src/garmin.c	Centralizes model and MTP capability lookup.
src/descriptor.c	Registers Garmin product descriptors.
include/libdivecomputer/parser.h	Declares the new public API.

Review details

Suppressed comments (1)

src/descriptor.c:548

• These descriptor IDs and product strings now duplicate garmin_models[] in src/garmin.c. A future FIT model can therefore be added to the parser/MTP table but omitted from the descriptor table (or vice versa), silently breaking the per-product exposure this change introduces. Please make one list the source of truth or add a consistency check.

	{"Garmin", "Descent™ G1 / G1 Solar", DC_FAMILY_GARMIN, 4005, DC_TRANSPORT_USBSTORAGE, NULL},
	{"Garmin", "Descent™ G2", DC_FAMILY_GARMIN, 4588, DC_TRANSPORT_USBSTORAGE, NULL},
	{"Garmin", "Descent™ Mk1", DC_FAMILY_GARMIN, 2859, DC_TRANSPORT_USBSTORAGE, NULL},
	{"Garmin", "Descent™ Mk1 APAC", DC_FAMILY_GARMIN, 2991, DC_TRANSPORT_USBSTORAGE, NULL},
	{"Garmin", "Descent™ Mk2(i)", DC_FAMILY_GARMIN, 3258, DC_TRANSPORT_USBSTORAGE, NULL},

• Files reviewed: 6/6 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🔵 Needs a closer look

Resolve Mk1 descriptor compatibility issues and document the new public parser API.

Review details

Suppressed comments (3)

Previously missed (3) — in code that hasn't changed since the last review.

src/descriptor.c:546

• The existing descentmk1 backend alias in examples/common.c:105-106 supplies model 0, and dctool_descriptor_search falls back to the first descriptor when no exact model exists (examples/common.c:287-299). With G1 now first, dctool -f descentmk1 ... selects model 4005 instead of the Mk1 descriptor it previously selected. Keep the Mk1 entry first or update that alias to model 2859.
  src/descriptor.c:546
• This changes the existing public product string from Descent Mk1 to Descent™ Mk1. dctool_descriptor_search performs an exact product-string match (examples/common.c:272-284), so existing callers or scripts using Garmin Descent Mk1 will no longer find a descriptor. Preserve the old string or add a compatibility alias instead of silently breaking lookup.
  include/libdivecomputer/parser.h:367
• This adds a public parser API, but the parser documentation set has no dc_parser_get_device_info.3 and doc/man/Makefile.am does not list it, unlike the existing parser functions. Add the man page and include it in MANPAGES so installed and distributed documentation describes the FIT-specific behavior and return contract.

• Files reviewed: 6/6 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

A moderate bounded-string safety issue and a project-file omission remain unresolved.

Review details

Suppressed comments (2)

examples/common.c:228

• This indexes name[1] and name[2] after seeing only that name[0] is a trademark lead byte. A user-supplied descriptor can end with a partial UTF-8 sequence, so this reads past the NUL terminator (and the analogous product indexing has the same hazard), causing undefined behavior for malformed or partial names. Compare the three-byte sequence with a bounded string operation instead.

		if ((unsigned char) product[0] == 0xE2 &&
		    (unsigned char) product[1] == 0x84 &&
		    ((unsigned char) product[2] == 0xA2 || (unsigned char) product[2] == 0xAE)) {

src/garmin-models.h:55

• The Automake source list includes this new header, but the checked-in MSVC project explicitly enumerates internal headers and still omits src\garmin-models.h (alongside src\garmin.h). Please add it to the project so Windows consumers can track/build the complete source set consistently.

extern const garmin_model_t garmin_models[];

• Files reviewed: 13/13 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[Copilot]

🔵 Needs a closer look

Preserve compatibility for existing Garmin descriptor names or document/version the breaking rename.

Review details

Suppressed comments (1)

src/descriptor.c:552

• This replaces the existing public product names (Descent Mk1 and Descent Mk2(i)/Mk3(i)(S)/G1/G2/X50i) with trademarked, per-model names. dctool_descriptor_search() matches product strings exactly (examples/common.c:272-285), so existing -d values and clients persisting dc_descriptor_get_product() will no longer resolve. Please preserve aliases or otherwise document/version this breaking rename.

#define GARMIN_DESCRIPTOR(name, id, mtp_capable) {"Garmin", name, DC_FAMILY_GARMIN, id, DC_TRANSPORT_USBSTORAGE, NULL},
	GARMIN_MODEL_LIST(GARMIN_DESCRIPTOR)

• Files reviewed: 11/11 changed files
• Comments generated: 0 new
• Review effort level: Lite