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

Fixed the choice set manager not handling failed preload choices correctly #2004

Conversation

NicoleYarroch
Copy link
Contributor

@NicoleYarroch NicoleYarroch commented May 26, 2021

Fixes #1941

Risk

This PR makes no API changes.

Testing Plan

  • I have verified that I have not introduced new warnings in this PR (or explain why below)
  • I have run the unit tests with this PR
  • I have tested this PR against Core and verified behavior (if applicable, if not applicable, explain why below).

Unit Tests

Test cases were added to SDLChoiceSetManagerSpec to test:

  • All the choice items failing to upload
  • Only one of the choice items failing to upload
  • Presenting the same choice item again after it fails to uploads
  • Presenting the same choice item again after it uploads successfully

Core Tests

  1. Presenting a choice set with a choice item that fails (this can be done by setting menuName with a string with 500+ characters). Then present the same choice set again. The failed choice item should be preloaded again.

Core version / branch / commit hash / module tested against:

  • SYNC 3.0 (Build 17276_DEVTEST)
  • Manticore (SDL Core v7.1.1)

HMI name / version / branch / commit hash / module tested against:

  • SYNC 3.0 (Build 17276_DEVTEST)
  • Manticore (Generic HMI v0.10.0)

Summary

Fixed the SDLChoiceSetManager sending a SDLPresentChoiceSetOperation if one more more of the choice set items fails to upload. The failed choice set items are no longer added to the list of preloaded choices so they can be sent to the module again if previous attempts to upload the choice item failed.

Changelog

Bug Fixes
  • Added error handling for failed choice set items to SDLChoiceSetManager.

Tasks Remaining:

  • Smoke test with SYNC 3

CLA

@NicoleYarroch NicoleYarroch self-assigned this May 26, 2021
@NicoleYarroch NicoleYarroch added bug A defect in the library manager-screen Relating to the manager layer - screen managers labels May 26, 2021
Signed-off-by: NicoleYarroch <[email protected]>
@codecov
Copy link

codecov bot commented May 26, 2021

Codecov Report

Merging #2004 (6ba82a8) into develop (e0c6c86) will decrease coverage by 0.82%.
The diff coverage is 100.00%.

❗ Current head 6ba82a8 differs from pull request most recent head a3fa94a. Consider uploading reports for the commit a3fa94a to get more accurate results

@@             Coverage Diff             @@
##           develop    #2004      +/-   ##
===========================================
- Coverage    86.07%   85.24%   -0.83%     
===========================================
  Files          441      441              
  Lines        22627    22617      -10     
===========================================
- Hits         19476    19280     -196     
- Misses        3151     3337     +186     

Copy link
Contributor

@joeljfischer joeljfischer left a comment

Choose a reason for hiding this comment

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

😅

SmartDeviceLink/private/SDLChoiceSetManager.m Outdated Show resolved Hide resolved
SmartDeviceLink/private/SDLChoiceSetManager.m Outdated Show resolved Hide resolved
SmartDeviceLink/private/SDLChoiceSetManager.m Outdated Show resolved Hide resolved
Signed-off-by: NicoleYarroch <[email protected]>
SmartDeviceLink/private/SDLChoiceSetManager.m Outdated Show resolved Hide resolved
SmartDeviceLink/private/SDLChoiceSetManager.m Outdated Show resolved Hide resolved
Signed-off-by: NicoleYarroch <[email protected]>
SmartDeviceLink/private/SDLChoiceSetManager.m Outdated Show resolved Hide resolved
SmartDeviceLink/private/SDLChoiceSetManager.m Outdated Show resolved Hide resolved
SmartDeviceLink/private/SDLChoiceSetManager.m Outdated Show resolved Hide resolved
SmartDeviceLink/private/SDLChoiceSetManager.m Outdated Show resolved Hide resolved
SmartDeviceLink/private/SDLChoiceSetManager.m Show resolved Hide resolved
SmartDeviceLinkTests/DevAPISpecs/SDLChoiceSetManagerSpec.m Outdated Show resolved Hide resolved
SmartDeviceLinkTests/DevAPISpecs/SDLChoiceSetManagerSpec.m Outdated Show resolved Hide resolved
NicoleYarroch and others added 5 commits June 3, 2021 15:17
@joeljfischer joeljfischer merged commit c76bbe1 into develop Jun 4, 2021
@joeljfischer joeljfischer deleted the bugfix/issue_1941_failed_preload_choice_not_handled_correctly branch June 4, 2021 14:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A defect in the library manager-screen Relating to the manager layer - screen managers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants