Skip to content

Conversation

@JulianKast
Copy link
Contributor

@JulianKast JulianKast commented May 21, 2021

Fixes #1676

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 VoiceCommandUpdateOperationTests.java

Core Tests

TEST:

Test 1:
VoiceCommandA = "command one"
VoiceCommandB = "command two"
Upload A and B on startup
VoiceCommandC = "command one"
VoiceCommandD = "command four"
Upload C and D in a menuSelection listener.
Verify Voice commands on HMI are "command one" and "command four"
Verify that when "command one" is selected the listener for VoiceCommandC is triggered.

Test2:
Upload A on startup
VoiceCommandA = "command one"
Upload C in a menuSelection listener.
VoiceCommandC = "command one"

Verify that when "command one" is selected the listener for VoiceCommandC is triggered.

Test3:
Upload A on startup
VoiceCommandA = "command one"
After A is uploaded, modify VoiceCommandA to have a new listener and upload it in something like a menuSelectionListener
Verify that when "command one" is selected the updated listener is triggered.

Test 4:
VoiceCommandA = "command one"
VoiceCommandB = "command two"
Upload A and B.
After A is uploaded, modify VoiceCommandA to have a new listener and upload it in something like a menuSelectionListener.
Verify that VoiceCommandB("command two") is removed form the hmi.
Verify that when "command one" is selected the updated listener is triggered.

Test 5:
Upload A on startup
VoiceCommandA = "command one"
After A is uploaded, modify VoiceCommandA to : "command one changed"
Verify that "Command one" is deleted and "Command one changed appears.
REPEAT THIS TEST WITH A NEW LISTENER AS WELL.

Test 6:
Upload voiceCommadA on startup
VoiceCommandA = "command one"
After A is uploaded, upload an empty list for voiceCommands
Verify that voiceCommadA is removed

Core version / branch / commit hash / module tested against: [INSERT]
HMI name / version / branch / commit hash / module tested against: [INSERT]

Summary

Modified logic in VoiceCommandManager to not check if voiceCommands are equal to old voice Commands since the new voiceCommands could have different listeners, and moved that logic to VoiceCommandUpdateOperation.
Now if you upload VoiceCommands and change a listener and upload them again, the command doesn't get resent to core, the listener just gets updated.

Also added logic to not upload identical voiceCommands.

Added clone

Changelog

Enhancements
  • Added logic to avoid deleting and setting identical voice commands
Bug Fixes
  • Added clone method to 'VoiceCommand" to allow VoiceCommandManger to properly clone voiceCommands.
  • Fixed scenario where voiceCommands couldn't be removed by sending an empty list.

CLA

Julian Kast added 2 commits May 21, 2021 12:21
@codecov
Copy link

codecov bot commented May 21, 2021

Codecov Report

Merging #1691 (106e3c8) into develop (2599121) will increase coverage by 0.12%.
The diff coverage is 61.17%.

Impacted file tree graph

@@              Coverage Diff              @@
##             develop    #1691      +/-   ##
=============================================
+ Coverage      54.17%   54.29%   +0.12%     
- Complexity      5335     5360      +25     
=============================================
  Files            555      555              
  Lines          24578    24637      +59     
  Branches        3106     3125      +19     
=============================================
+ Hits           13314    13377      +63     
+ Misses         10106    10094      -12     
- Partials        1158     1166       +8     
Impacted Files Coverage Δ
...icelink/transport/MultiplexBluetoothTransport.java 4.26% <0.00%> (-0.03%) ⬇️
...martdevicelink/transport/SdlBroadcastReceiver.java 3.21% <0.00%> (-0.03%) ⬇️
...artdevicelink/transport/utl/SdlDeviceListener.java 10.34% <0.00%> (ø)
...tdevicelink/managers/screen/menu/VoiceCommand.java 75.00% <28.57%> (+14.13%) ⬆️
...agers/screen/menu/VoiceCommandUpdateOperation.java 76.37% <70.83%> (-0.48%) ⬇️
.../managers/screen/menu/BaseVoiceCommandManager.java 83.20% <82.50%> (+17.24%) ⬆️
...nk/managers/audio/AudioDecoderCompatOperation.java 75.00% <0.00%> (-4.55%) ⬇️
...ink/managers/screen/BaseTextAndGraphicManager.java 64.16% <0.00%> (+0.41%) ⬆️
...rtdevicelink/streaming/video/SdlRemoteDisplay.java 51.21% <0.00%> (+1.21%) ⬆️

@bilal-alsharifi bilal-alsharifi merged commit 611d350 into develop Jun 2, 2021
@bilal-alsharifi bilal-alsharifi deleted the bugfix/issue_1676 branch June 2, 2021 15:34
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.

3 participants