Skip to content

Fix soft button object invalid states #1776

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.

Sign up for GitHub

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

Merged
merged 20 commits into from
Feb 3, 2022
Merged

Fix soft button object invalid states #1776

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
2445b37
Resolve #1774
noah-livio Dec 7, 2021
72e24b5
Add tests
noah-livio Dec 7, 2021
c9da956
Update Javadoc with throws annotations
noah-livio Dec 7, 2021
0aebb4e
Removing throwing of IllegalStateException
noah-livio Dec 14, 2021
b00cea5
Remove unused import
noah-livio Dec 14, 2021
8413d94
Make changes discussed in #1776 review
noah-livio Dec 15, 2021
f95aec3
Fix a small typo in the unit test javadoc
noah-livio Dec 15, 2021
648bdc3
Cleanup code and modify tests
noah-livio Dec 17, 2021
9a82059
Move states assignment back to original line
noah-livio Dec 17, 2021
053ac1b
Add duplicate name check to setStates()
noah-livio Dec 17, 2021
9d47512
Add AssertionErrors as per iOS alignment
noah-livio Jan 7, 2022
386027f
Remove redundant conditions and commented code
noah-livio Jan 7, 2022
75d1e1e
Add AssertionError to SoftButtonState
noah-livio Jan 7, 2022
d15498f
Add test for initialStateName check
noah-livio Jan 10, 2022
34c0622
Revert addtion of AssertionErrors
noah-livio Jan 11, 2022
528769f
Update SoftButtonManagerTests test
noah-livio Jan 20, 2022
e183f35
Update ScreenManagerTests test
noah-livio Jan 20, 2022
666f99a
Fixes from PR feedback
noah-livio Jan 26, 2022
14b7569
Review and standup feedback fixes
noah-livio Jan 28, 2022
39efd8e
PR feedback change
noah-livio Feb 1, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Fixes from PR feedback
  • Loading branch information
@noah-livio
noah-livio committed Jan 26, 2022
commit 666f99a9c688b90f022cacc894255f31712795d3
13 changes: 10 additions & 3 deletions base/src/main/java/com/smartdevicelink/managers/screen/SoftButtonObject.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@

import androidx.annotation.NonNull;

import com.livio.BuildConfig;
import com.smartdevicelink.managers.file.filetypes.SdlArtwork;
import com.smartdevicelink.proxy.rpc.OnButtonEvent;
import com.smartdevicelink.proxy.rpc.OnButtonPress;
Expand Down Expand Up @@ -283,8 +282,16 @@ public void setStates(@NonNull List<SoftButtonState> states) {
DebugTool.logError(TAG, "A SoftButtonObject must have states with different names.");
return;
}
if (states.isEmpty()) {
DebugTool.logError(TAG, "A SoftButtonState list must contain at least one state");

boolean hasStateWithCurrentName = false;
for (SoftButtonState state : states) {
if(state.getName().equals(currentStateName)) {
hasStateWithCurrentNameName = true;
Copy link
Contributor

@RHenigan RHenigan Jan 28, 2022

Choose a reason for hiding this comment

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

Suggested change
hasStateWithCurrentNameName = true;
hasStateWithCurrentName = true;

Copy link
Contributor Author

@noah-livio noah-livio Jan 28, 2022

Choose a reason for hiding this comment

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

Good catch, and that'll be easy to fix

Copy link
Contributor

@RHenigan RHenigan Jan 28, 2022

Choose a reason for hiding this comment

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

Also to avoid confusion for the developer we will still want to set the states in the case the currentState is not present in the list, but we will still want to log the error.

We should test and confirm that the SoftButtonManager can handle this error case (trying to upload a SoftButton where the currentState is not in the list)

break;
}
}
if (!hasStateWithCurrentName) {
DebugTool.logError(TAG, "A SoftButtonObject must have a state with currentStateName.");
return;
}

Expand Down
1 change: 0 additions & 1 deletion base/src/main/java/com/smartdevicelink/managers/screen/SoftButtonState.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@

import androidx.annotation.NonNull;

import com.livio.BuildConfig;
import com.smartdevicelink.managers.file.filetypes.SdlArtwork;
import com.smartdevicelink.proxy.rpc.SoftButton;
import com.smartdevicelink.proxy.rpc.enums.SoftButtonType;
Expand Down