Skip to content
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

Fix Copilot unrecognized fields serialization #6821

Merged
merged 1 commit into from
Jan 12, 2023

Conversation

Guardiola31337
Copy link
Contributor

Description

While testing we found that DirectionsResponse fields from Copilot History files were not properly serialized:

  • field names were incorrect (camelCase vs snake_case)
  • unrecognized fields not serialized (also obfuscated if Proguard was enabled)

E.g. 👀

"incidents": [
                {
                  "affectedRoadNames": [
                    "A-3/E 901/Autovía del Este",
                    "A-3/E 901"
                  ],
                  "alertcCodes": [
                    703
                  ],
                  "congestion": {
                    "value": 101
                  },
                  "countryCodeAlpha2": "ES",
                  "countryCodeAlpha3": "ESP",
                  "creationTime": "2023-01-12T13:40:30Z",
                  "description": "A-3: obras de mantenimiento entre 68 M-328 y 111 CUV-3131",
                  "endTime": "2023-01-12T19:40:00Z",
                  "geometryIndexEnd": 273,
                  "geometryIndexStart": 76,
                  "id": "8907455158012201",
                  "impact": "low",
                  "lanesBlocked": [],
                  "longDescription": "Obras de mantenimiento en la ambos sentidos A-3 entre 68 M-328 y 111 CUV-3131.",
                  "startTime": "2023-01-11T22:40:00Z",
                  "type": "construction",
                  "unrecognized": {
                    "length": {
                      "a": 39194
                    },
                    "south": {
                      "a": 39.904338
                    },
                    "west": {
                      "a": -3.086498
                    },
                    "north": {
                      "a": 40.068935
                    },
                    "east": {
                      "a": -2.717774
                    }
                  }
                }
]

This PR fixes it 👀

"incidents": [
                {
                  "length": 39194,
                  "south": 39.904338,
                  "west": -3.086498,
                  "north": 40.068935,
                  "east": -2.717774,
                  "id": "8907455158012201",
                  "type": "construction",
                  "congestion": {
                    "value": 101
                  },
                  "description": "A-3: obras de mantenimiento entre 68 M-328 y 111 CUV-3131",
                  "long_description": "Obras de mantenimiento en la ambos sentidos A-3 entre 68 M-328 y 111 CUV-3131.",
                  "impact": "low",
                  "alertc_codes": [
                    703
                  ],
                  "geometry_index_start": 914,
                  "geometry_index_end": 972,
                  "creation_time": "2023-01-12T14:25:31Z",
                  "start_time": "2023-01-11T22:40:00Z",
                  "end_time": "2023-01-12T19:40:00Z",
                  "iso_3166_1_alpha2": "ES",
                  "iso_3166_1_alpha3": "ESP",
                  "lanes_blocked": [],
                  "affected_road_names": [
                    "A-3/E 901/Autovía del Este",
                    "A-3/E 901"
                  ]
                }
]

@Guardiola31337 Guardiola31337 added needs backporting Requires cherry-picking to a currently running release branch skip changelog Should not be added into version changelog. labels Jan 12, 2023
@Guardiola31337 Guardiola31337 self-assigned this Jan 12, 2023
@github-actions
Copy link

Changelog

Features

  • Introduced ViewStyleCustomization.infoPanelGuidelineMaxPosPercent that allows customization of the NavigationView InfoPanel bottom guideline maximum position. Increased default value to 50%. #6792

Bug fixes and improvements

  • Fixed an issue with NavigationView that caused info panel to shrink in landscape mode with a full screen theme. #6780

  • ⚠️ Updated the NavigationView default navigation puck asset. #6678

    Previous puck can be restored by injecting LocationPuck2D with the bearingImage set to com.mapbox.navigation.ui.maps.R.drawable.mapbox_navigation_puck_icon drawable:

    navigationView.customizeViewStyles {
        locationPuckOptions = LocationPuckOptions.Builder(context)
            .defaultPuck(
                LocationPuck2D(
                    bearingImage = ContextCompat.getDrawable(
                        context,
                        com.mapbox.navigation.ui.maps.R.drawable.mapbox_navigation_puck_icon,
                    )
                )
            )
            .idlePuck(regularPuck(context))
            .build()
    }
  • Fixed an issue where the first voice instruction might have been played twice. #6766

  • Added guarantees that route progress with RouteProgress#currentState == OFF_ROUTE arrives earlier than NavigationRerouteController#reroute is called. #6764

  • Improved MapboxNavigation#startReplayTripSession and ReplayRouteSession so that the previous trip session does not need to be stopped. ⚠️ ReplayRouteSession#onDetached removed the call to stopTripSession. #6817

  • Introduced NavigationViewListener.onSpeedInfoClicked that would be triggered when MapboxSpeedInfoView is clicked upon. #6770

  • Each newly instantiated MapboxRouteArrowView class will initialize the layers with the provided options on the first render call. Previously this would only be done if the layers hadn't already been initialized. #6466

  • Fixed a rare java.lang.NullPointerException: Attempt to read from field 'SpeechAnnouncement PlayCallback.announcement' on a null object reference crash in PlayCallback.getAnnouncement. #6760

  • Fixed standalone MapboxManeuverView appearance when the app also integrates Drop-In UI. #6774

Known issues ⚠️

Other changes

Android Auto Changelog

Features

Bug fixes and improvements

Comment on lines +404 to +405
.registerTypeAdapterFactory(DirectionsAdapterFactory.create())
.registerTypeAdapter(Point::class.java, PointAsCoordinatesTypeAdapter())
Copy link
Contributor

@VysotskiVadim VysotskiVadim Jan 12, 2023

Choose a reason for hiding this comment

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

I think it would be nice if mapbox-java exposed a single point for adapter, so that:

  1. other users could use it in their projects with 1 extra line of code
  2. if we add new adapters, we don't need to add them to copilot too

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great idea! Could you cut a ticket in mapbox-java when you have a chance? 🙏

@codecov
Copy link

codecov bot commented Jan 12, 2023

Codecov Report

Merging #6821 (b3ca4f9) into main (d8eeb04) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##               main    #6821   +/-   ##
=========================================
  Coverage     72.63%   72.63%           
  Complexity     5568     5568           
=========================================
  Files           780      780           
  Lines         30101    30104    +3     
  Branches       3556     3556           
=========================================
+ Hits          21864    21867    +3     
  Misses         6809     6809           
  Partials       1428     1428           
Impacted Files Coverage Δ
...com/mapbox/navigation/copilot/MapboxCopilotImpl.kt 92.37% <100.00%> (+0.10%) ⬆️

@Guardiola31337 Guardiola31337 merged commit 6e94853 into main Jan 12, 2023
@Guardiola31337 Guardiola31337 deleted the pg-copilot-unrecognized-fields-obfuscated branch January 12, 2023 15:40
Guardiola31337 added a commit that referenced this pull request Jan 12, 2023
* fix copilot unrecognized fields serialization (#6821)

* [copilot] add proguard rule to prevent obfuscation of route properties (#6762)
Guardiola31337 added a commit that referenced this pull request Jan 12, 2023
* fix copilot unrecognized fields serialization (#6821)

* [copilot] add proguard rule to prevent obfuscation of route properties (#6762)
Guardiola31337 added a commit that referenced this pull request Jan 12, 2023
* fix copilot unrecognized fields serialization (#6821)

* [copilot] add proguard rule to prevent obfuscation of route properties (#6762)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs backporting Requires cherry-picking to a currently running release branch skip changelog Should not be added into version changelog.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants