Skip to content

Conversation

@JulianKast
Copy link
Contributor

@JulianKast JulianKast commented May 10, 2021

Fixes #1682

This PR is ready for review.

Risk

This PR makes minor 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

Unit test were added in MenuManagerTests and ChoiceSetManagerTests

Core Tests

  • Tested that Core 7.1.1, sdl_hmi v5.4.0 works with completely unique menu.
  • Tested that Core 7.1.1, sdl_hmi v5.4.0 rejects complete duplicates in menu.
  • Tested that Core 7.1.1, sdl_hmi v5.4.0 with one version back sdl_hmi adds unique identifiers to primary text duplicates with uniquing secondaryText.
  • Tested that Core 7.1.1, sdl_hmi v5.4.0 rejects complete duplicates in choices.
  • Tested that Core 7.1.1, sdl_hmi v5.4.0 works with completely unique choices.
  • Tested that Core 7.1.1, generic_hmi v0.10.0 (altered to not support Choice.secondaryText and Choice.tertiaryText) adds unique identifiers to choices with primary text duplicates with uniquing secondaryText.
  • Tested that Sync 3.0 works with completely unique menu.
  • Tested that Sync 3.0 reject menu if there are complete duplicates in menu.
  • Tested that Sync 3.0 adds unique identifiers to duplicate primary text.
  • Tested that Sync 3.0 rejects complete duplicates in choices.
  • Tested that Sync 3.0 works with completely unique choices.
  • Tested that Sync 3.0 continues to add unique identifiers to duplicate primary text.

Core version / branch / commit hash / module tested against: v7.1.1, Sync 3.0
HMI name / version / branch / commit hash / module tested against: generic_HMI v0.10.0 (current version), sdl_hmi v5.4.0 (one version back so that secondary / tertiary text isn't supported in menus), Sync 3.0

Summary

This PR updates the screen manager (menu manager and choice manager) to check the uniqueness of cells based on available screen properties on RPC v7.1+.

Changelog

Bug Fixes
  • On RPC v7.1+ connections, we will no longer display duplicate menu or choice cells if some property of the cell is different but that property isn't actually displayed on the head unit.

CLA

@JulianKast JulianKast changed the base branch from master to develop May 10, 2021 13:47
@codecov
Copy link

codecov bot commented May 10, 2021

Codecov Report

Merging #1684 (108f315) into develop (889058f) will increase coverage by 0.13%.
The diff coverage is 78.02%.

Impacted file tree graph

@@              Coverage Diff              @@
##             develop    #1684      +/-   ##
=============================================
+ Coverage      54.22%   54.35%   +0.13%     
- Complexity      5351     5392      +41     
=============================================
  Files            555      555              
  Lines          24622    24702      +80     
  Branches        3122     3144      +22     
=============================================
+ Hits           13351    13427      +76     
+ Misses         10111    10108       -3     
- Partials        1160     1167       +7     
Impacted Files Coverage Δ
...anagers/screen/choiceset/BaseChoiceSetManager.java 43.88% <69.76%> (+4.01%) ⬆️
...vicelink/managers/screen/menu/BaseMenuManager.java 48.92% <85.41%> (+3.03%) ⬆️
...vicelink/managers/screen/choiceset/ChoiceCell.java 81.33% <0.00%> (+8.00%) ⬆️

Julian Kast added 27 commits May 10, 2021 16:32
… duplicate menu cells if some property of the cell is different but that property isn't actually displayed on the head unit.
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.

Choice Cells and Menu Cells do not take which properties are available into account for uniqueness

3 participants