Skip to content

Refactor ChoiceSetManager to Fix several bugs#1722

Merged
RHenigan merged 52 commits intodevelopfrom
bugfix/issue_1717_1718
Sep 20, 2021
Merged

Refactor ChoiceSetManager to Fix several bugs#1722
RHenigan merged 52 commits intodevelopfrom
bugfix/issue_1717_1718

Conversation

@RHenigan
Copy link
Copy Markdown
Contributor

@RHenigan RHenigan commented Jul 26, 2021

Fixes #1717, #1718

This PR is ready for review.

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).
  • I have tested Android, Java SE, and Java EE

Unit Tests

Updated unit test for new operation

Core Tests

Tested send in a presentation, then sending another presentation when the first has a button pressed
Smoke Tests for choiceset manager

Core version / branch / commit hash / module tested against: Manticore (SDL v7.1.1)
HMI name / version / branch / commit hash / module tested against: (Generic HMI v0.10.0)

Summary

  1. This PR rewrites the preload / present operations into one operation to solve some queueing problems. The operation will now figure out what it has to do based on how it is initialized.
  2. This PR changes cancelation behavior. Before if you sent a present, then another present, the manager would try to cancel previous ones. Now they will be queued and the dev can cancel previous ones if desired.
  3. This PR fixes strange deletion behavior. Before if a delete came in while a preload / present was happening, the manager would try to delete choices from the upload. Now it will wait until the delete operation comes around and delete at that time. This allows proper behavior around preload -> present -> delete situations.

CLA

@RHenigan RHenigan changed the title Bugfix/issue 1717 1718 Refactor ChoiceSetManager to Fix several bugs Jul 26, 2021
@codecov
Copy link
Copy Markdown

codecov Bot commented Jul 26, 2021

Codecov Report

Merging #1722 (27184db) into develop (732801d) will decrease coverage by 0.14%.
The diff coverage is 32.45%.

Impacted file tree graph

@@              Coverage Diff              @@
##             develop    #1722      +/-   ##
=============================================
- Coverage      54.13%   53.99%   -0.15%     
- Complexity      5429     5430       +1     
=============================================
  Files            555      554       -1     
  Lines          25082    25214     +132     
  Branches        3232     3278      +46     
=============================================
+ Hits           13579    13614      +35     
- Misses         10313    10381      +68     
- Partials        1190     1219      +29     
Impacted Files Coverage Δ
...agers/screen/choiceset/DeleteChoicesOperation.java 16.17% <2.56%> (-14.13%) ⬇️
...anagers/screen/choiceset/BaseChoiceSetManager.java 41.37% <9.52%> (-2.15%) ⬇️
...reen/choiceset/PreloadPresentChoicesOperation.java 35.77% <35.77%> (ø)
...vicelink/managers/screen/choiceset/ChoiceCell.java 77.50% <81.81%> (-3.84%) ⬇️
...rtdevicelink/streaming/video/SdlRemoteDisplay.java 50.00% <0.00%> (-1.22%) ⬇️
...a/com/smartdevicelink/managers/ManagerUtility.java 77.61% <0.00%> (+1.49%) ⬆️
...nk/managers/audio/AudioDecoderCompatOperation.java 79.54% <0.00%> (+4.54%) ⬆️

@RHenigan RHenigan marked this pull request as ready for review July 29, 2021 14:30
Copy link
Copy Markdown
Contributor

@JulianKast JulianKast left a comment

Choose a reason for hiding this comment

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

@RHenigan after my first review, I noticed a few small things.

Copy link
Copy Markdown
Contributor

@JulianKast JulianKast left a comment

Choose a reason for hiding this comment

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

minor updates needed

Robert Henigan and others added 5 commits September 16, 2021 16:03
…et/PreloadPresentChoicesOperation.java

Co-authored-by: Julian Kast <Julian.kast@livio.io>
…et/PreloadPresentChoicesOperation.java

Co-authored-by: Julian Kast <Julian.kast@livio.io>
Co-authored-by: Julian Kast <Julian.kast@livio.io>
@RHenigan RHenigan merged commit 470e703 into develop Sep 20, 2021
@RHenigan RHenigan deleted the bugfix/issue_1717_1718 branch September 20, 2021 18:06
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.

2 participants