Skip to content

Garmin: Add MTP Support for More Models. #82

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: Subsurface-DS9
Choose a base branch
from

Conversation

mikeller
Copy link
Member

@mikeller mikeller commented Jun 5, 2025

Add MTP support for more of the newer Garmin Descent models.

@mikeller mikeller requested a review from Copilot June 5, 2025 22:02
Copilot

This comment was marked as outdated.

@mikeller mikeller force-pushed the add_garmin_mtp_supported_models branch from bcc993d to 2127a7e Compare June 5, 2025 22:07
@mikeller mikeller requested a review from Copilot June 5, 2025 22:08
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Add MTP support for additional Garmin Descent models by centralizing model metadata and leveraging a shared model list in both the parser and device code.

  • Introduce a garmin_model_t struct (including an mtp_capable flag) and declare a global garmin_models array in the header.
  • Update src/garmin_parser.c to use the shared garmin_models array instead of a local static list.
  • Enhance MTP detection logic in src/garmin.c to iterate over garmin_models and skip non-MTP-capable devices.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
src/garmin_parser.c Removed local models[] array, replaced with garmin_models lookup.
src/garmin.h Added garmin_model_t definition (with mtp_capable) and extern array.
src/garmin.c Defined garmin_models with MTP flags, updated product-ID checks.
Comments suppressed due to low confidence (2)

src/garmin_parser.c:1667

  • [nitpick] The loop index variable 'i' is not descriptive. Consider renaming it to something like 'model_index' for clarity.
unsigned i;

src/garmin.h:39

  • Consider adding a comment explaining what the 'mtp_capable' flag represents and how it should be used, to improve code self-documentation.
    bool mtp_capable;


bool mtp_capable = false;
for (unsigned j = 0; garmin_models[j].name; j++) {
if ((garmin_models[j].id | 0x4000) == rawdevices[i].device_entry.product_id) {
Copy link
Preview

Copilot AI Jun 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Magic number 0x4000 is used to derive the USB product ID flag. Consider defining a named constant (e.g., USB_MTP_FLAG) to improve readability and maintainability.

Suggested change
if ((garmin_models[j].id | 0x4000) == rawdevices[i].device_entry.product_id) {
if ((garmin_models[j].id | USB_MTP_FLAG) == rawdevices[i].device_entry.product_id) {

Copilot uses AI. Check for mistakes.

@mikeller mikeller force-pushed the add_garmin_mtp_supported_models branch from 2127a7e to 6a2fbe4 Compare June 8, 2025 04:15
Add MTP support for more of the newer Garmin Descent models.

Signed-off-by: Michael Keller <[email protected]>
@mikeller mikeller force-pushed the add_garmin_mtp_supported_models branch from 6a2fbe4 to fc2ff86 Compare June 14, 2025 07:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant