Skip to content

Commit 3d1e54e

Browse files
author
Robert Henigan
authored
Merge pull request #1776 from noah-livio/bugfix/issue_1774
Fix soft button object invalid states
2 parents 454e1e6 + 39efd8e commit 3d1e54e

3 files changed

Lines changed: 174 additions & 24 deletions

File tree

‎android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/ScreenManagerTests.java‎

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -213,14 +213,15 @@ public void testSettingSoftButtonId() {
213213

214214
@Test
215215
public void testAssigningIdsToSoftButtonObjects() {
216+
SoftButtonState defaultState = new SoftButtonState("default", "hi", null);
216217
SoftButtonObject sbo1, sbo2, sbo3, sbo4, sbo5;
217218

218219
// Case 1 - don't set id for any button (Manager should set ids automatically starting from 1 and up)
219-
sbo1 = new SoftButtonObject(null, Collections.EMPTY_LIST, null, null);
220-
sbo2 = new SoftButtonObject(null, Collections.EMPTY_LIST, null, null);
221-
sbo3 = new SoftButtonObject(null, Collections.EMPTY_LIST, null, null);
222-
sbo4 = new SoftButtonObject(null, Collections.EMPTY_LIST, null, null);
223-
sbo5 = new SoftButtonObject(null, Collections.EMPTY_LIST, null, null);
220+
sbo1 = new SoftButtonObject(null, defaultState, null);
221+
sbo2 = new SoftButtonObject(null, defaultState, null);
222+
sbo3 = new SoftButtonObject(null, defaultState, null);
223+
sbo4 = new SoftButtonObject(null, defaultState, null);
224+
sbo5 = new SoftButtonObject(null, defaultState, null);
224225
screenManager.checkAndAssignButtonIds(Arrays.asList(sbo1, sbo2, sbo3, sbo4, sbo5), BaseScreenManager.ManagerLocation.SOFTBUTTON_MANAGER);
225226
assertEquals("SoftButtonObject id doesn't match the expected value", 1, sbo1.getButtonId());
226227
assertEquals("SoftButtonObject id doesn't match the expected value", 2, sbo2.getButtonId());
@@ -230,15 +231,15 @@ public void testAssigningIdsToSoftButtonObjects() {
230231

231232

232233
// Case 2 - Set ids for all buttons (Manager shouldn't alter the ids set by developer)
233-
sbo1 = new SoftButtonObject(null, Collections.EMPTY_LIST, null, null);
234+
sbo1 = new SoftButtonObject(null, defaultState, null);
234235
sbo1.setButtonId(100);
235-
sbo2 = new SoftButtonObject(null, Collections.EMPTY_LIST, null, null);
236+
sbo2 = new SoftButtonObject(null, defaultState, null);
236237
sbo2.setButtonId(200);
237-
sbo3 = new SoftButtonObject(null, Collections.EMPTY_LIST, null, null);
238+
sbo3 = new SoftButtonObject(null, defaultState, null);
238239
sbo3.setButtonId(300);
239-
sbo4 = new SoftButtonObject(null, Collections.EMPTY_LIST, null, null);
240+
sbo4 = new SoftButtonObject(null, defaultState, null);
240241
sbo4.setButtonId(400);
241-
sbo5 = new SoftButtonObject(null, Collections.EMPTY_LIST, null, null);
242+
sbo5 = new SoftButtonObject(null, defaultState, null);
242243
sbo5.setButtonId(500);
243244
screenManager.checkAndAssignButtonIds(Arrays.asList(sbo1, sbo2, sbo3, sbo4, sbo5), BaseScreenManager.ManagerLocation.SOFTBUTTON_MANAGER);
244245
assertEquals("SoftButtonObject id doesn't match the expected value", 100, sbo1.getButtonId());
@@ -249,13 +250,13 @@ public void testAssigningIdsToSoftButtonObjects() {
249250

250251

251252
// Case 3 - Set ids for some buttons (Manager shouldn't alter the ids set by developer. And it should assign ids for the ones that don't have id)
252-
sbo1 = new SoftButtonObject(null, Collections.EMPTY_LIST, null, null);
253+
sbo1 = new SoftButtonObject(null, defaultState, null);
253254
sbo1.setButtonId(50);
254-
sbo2 = new SoftButtonObject(null, Collections.EMPTY_LIST, null, null);
255-
sbo3 = new SoftButtonObject(null, Collections.EMPTY_LIST, null, null);
256-
sbo4 = new SoftButtonObject(null, Collections.EMPTY_LIST, null, null);
255+
sbo2 = new SoftButtonObject(null, defaultState, null);
256+
sbo3 = new SoftButtonObject(null, defaultState, null);
257+
sbo4 = new SoftButtonObject(null, defaultState, null);
257258
sbo4.setButtonId(100);
258-
sbo5 = new SoftButtonObject(null, Collections.EMPTY_LIST, null, null);
259+
sbo5 = new SoftButtonObject(null, defaultState, null);
259260
screenManager.checkAndAssignButtonIds(Arrays.asList(sbo1, sbo2, sbo3, sbo4, sbo5), BaseScreenManager.ManagerLocation.SOFTBUTTON_MANAGER);
260261
assertEquals("SoftButtonObject id doesn't match the expected value", 50, sbo1.getButtonId());
261262
assertEquals("SoftButtonObject id doesn't match the expected value", 101, sbo2.getButtonId());

‎android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/SoftButtonManagerTests.java‎

Lines changed: 118 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
import java.util.Collections;
4545
import java.util.List;
4646

47+
4748
import static junit.framework.TestCase.assertEquals;
4849
import static junit.framework.TestCase.assertFalse;
4950
import static junit.framework.TestCase.assertNull;
@@ -350,8 +351,8 @@ public void onEvent(SoftButtonObject softButtonObject, OnButtonEvent onButtonEve
350351
softButtonStateList.add(softButtonState1);
351352
softButtonStateList2.add(softButtonState1);
352353
softButtonStateList2.add(softButtonState2);
353-
softButtonObject1 = new SoftButtonObject("hi", softButtonStateList, "Hi", null);
354-
softButtonObject2 = new SoftButtonObject("hi", softButtonStateList2, "Hi", null);
354+
softButtonObject1 = new SoftButtonObject("hi", softButtonStateList, softButtonStateList.get(0).getName(), null);
355+
softButtonObject2 = new SoftButtonObject("hi", softButtonStateList2, softButtonStateList2.get(0).getName(), null);
355356
assertNotEquals(softButtonObject1, softButtonObject2);
356357

357358
// Case 5: SoftButtonStates are not the same, assertFalse
@@ -365,8 +366,8 @@ public void onEvent(SoftButtonObject softButtonObject, OnButtonEvent onButtonEve
365366
assertNotEquals(softButtonObject1, softButtonObject2);
366367

367368
// Case 7: SoftButtonObject currentStateName not same, assertFalse
368-
softButtonObject1 = new SoftButtonObject("hi", softButtonStateList, "Hi", null);
369-
softButtonObject2 = new SoftButtonObject("hi", softButtonStateList, "Hi2", null);
369+
softButtonObject1 = new SoftButtonObject("hi", softButtonStateList2, softButtonStateList2.get(0).getName(), null);
370+
softButtonObject2 = new SoftButtonObject("hi", softButtonStateList2, softButtonStateList2.get(1).getName(), null);
370371
assertNotEquals(softButtonObject1, softButtonObject2);
371372
}
372373

@@ -402,4 +403,117 @@ public void testSoftButtonStateEquals() {
402403
softButtonState2 = new SoftButtonState("object1-state1", "o1s1", artwork1);
403404
assertEquals(softButtonState1, softButtonState2);
404405
}
406+
407+
/**
408+
* Test constructing SoftButtonObject with an empty state list
409+
*/
410+
@Test
411+
public void testConstructSoftButtonObjectWithEmptyStateList() {
412+
List<SoftButtonState> stateList = new ArrayList<>();
413+
SoftButtonObject softButtonObject = new SoftButtonObject("hello_there", stateList, "general_kenobi", null);
414+
assertNull(softButtonObject.getStates());
415+
}
416+
417+
/**
418+
* Test constructing SoftButtonObject with an nonempty state list
419+
*/
420+
@Test
421+
public void testConstructSoftButtonObjectWithNonEmptyStateList() {
422+
List<SoftButtonState> stateList = new ArrayList<>();
423+
SoftButtonState softButtonState = new SoftButtonState("general_kenobi", "General Kenobi", null);
424+
stateList.add(softButtonState);
425+
SoftButtonObject softButtonObject = new SoftButtonObject("hello_there", stateList, "general_kenobi", null);
426+
assertEquals(stateList, softButtonObject.getStates());
427+
}
428+
429+
/**
430+
* Test constructing SoftButtonObject with an invalid initialStateName
431+
*/
432+
@Test
433+
public void testConstructSoftButtonObjectWithInvalidInitialStateName() {
434+
List<SoftButtonState> stateList = new ArrayList<>();
435+
SoftButtonState softButtonState = new SoftButtonState("general_kenobi", "General Kenobi", null);
436+
stateList.add(softButtonState);
437+
SoftButtonObject softButtonObject = new SoftButtonObject("hello_there", stateList, "hello_there", null);
438+
assertNull(softButtonObject.getStates());
439+
}
440+
441+
/**
442+
* Test assigning an empty state list to existing SoftButtonObject
443+
*/
444+
@Test
445+
public void testAssignEmptyStateListToSoftButtonObject() {
446+
List<SoftButtonState> nonEmptyStateList = new ArrayList<>();
447+
List<SoftButtonState> emptyStateList = new ArrayList<>();
448+
SoftButtonState softButtonState = new SoftButtonState("general_kenobi", "General Kenobi", null);
449+
nonEmptyStateList.add(softButtonState);
450+
451+
SoftButtonObject softButtonObject = new SoftButtonObject("hello_there", nonEmptyStateList, "general_kenobi", null);
452+
453+
softButtonObject.setStates(emptyStateList);
454+
assertEquals(nonEmptyStateList, softButtonObject.getStates());
455+
}
456+
457+
/**
458+
* Test assigning a state list with the current state to existing SoftButtonObject
459+
*/
460+
@Test
461+
public void testAssignStateListWithCurrentStateToSoftButtonObject() {
462+
List<SoftButtonState> stateList1 = new ArrayList<>();
463+
SoftButtonState softButtonState1 = new SoftButtonState("hello_there", "Hello there", null);
464+
stateList1.add(softButtonState1);
465+
466+
List<SoftButtonState> stateList2 = new ArrayList<>();
467+
SoftButtonState softButtonState2 = new SoftButtonState("general_kenobi", "General Kenobi", null);
468+
stateList2.add(softButtonState1);
469+
stateList2.add(softButtonState2);
470+
471+
SoftButtonObject softButtonObject = new SoftButtonObject("general_kenobi", stateList1, "hello_there", null);
472+
473+
softButtonObject.setStates(stateList2);
474+
475+
assertEquals(stateList2, softButtonObject.getStates());
476+
}
477+
478+
/**
479+
* Test assigning a state list without the current state to existing SoftButtonObject
480+
*/
481+
@Test
482+
public void testAssignStateListWithoutCurrentStateToSoftButtonObject() {
483+
List<SoftButtonState> stateList1 = new ArrayList<>();
484+
SoftButtonState softButtonState1 = new SoftButtonState("hello_there", "Hello there", null);
485+
stateList1.add(softButtonState1);
486+
487+
List<SoftButtonState> stateList2 = new ArrayList<>();
488+
SoftButtonState softButtonState2 = new SoftButtonState("general_kenobi", "General Kenobi", null);
489+
stateList2.add(softButtonState2);
490+
491+
SoftButtonObject softButtonObject = new SoftButtonObject("general_kenobi", stateList1, "hello_there", null);
492+
493+
softButtonObject.setStates(stateList2);
494+
495+
assertEquals(stateList2, softButtonObject.getStates());
496+
}
497+
498+
/**
499+
* Test assigning a state list with states that have the same name to existing SoftButtonObject
500+
*/
501+
@Test
502+
public void testAssignSameNameStateListToSoftButtonObject() {
503+
List<SoftButtonState> stateListUnique = new ArrayList<>();
504+
SoftButtonState softButtonState1 = new SoftButtonState("hello_there", "Hello there", null);
505+
stateListUnique.add(softButtonState1);
506+
507+
List<SoftButtonState> stateListDuplicateNames = new ArrayList<>();
508+
SoftButtonState softButtonState2 = new SoftButtonState("general_kenobi", "General Kenobi", null);
509+
stateListDuplicateNames.add(softButtonState2);
510+
SoftButtonState softButtonState3 = new SoftButtonState("general_kenobi", "General Kenobi Again", null);
511+
stateListDuplicateNames.add(softButtonState3);
512+
513+
SoftButtonObject softButtonObject = new SoftButtonObject("general_kenobi", stateListUnique, "hello_there", null);
514+
515+
softButtonObject.setStates(stateListDuplicateNames);
516+
517+
assertEquals(stateListUnique, softButtonObject.getStates());
518+
}
405519
}

‎base/src/main/java/com/smartdevicelink/managers/screen/SoftButtonObject.java‎

Lines changed: 40 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@
3838
import com.smartdevicelink.proxy.rpc.OnButtonPress;
3939
import com.smartdevicelink.proxy.rpc.SoftButton;
4040
import com.smartdevicelink.util.DebugTool;
41-
4241
import java.util.Collections;
4342
import java.util.List;
4443

@@ -73,12 +72,24 @@ public class SoftButtonObject implements Cloneable {
7372
*/
7473
public SoftButtonObject(@NonNull String name, @NonNull List<SoftButtonState> states, @NonNull String initialStateName, OnEventListener onEventListener) {
7574

76-
// Make sure there aren't two states with the same name
77-
if (hasTwoStatesOfSameName(states)) {
78-
DebugTool.logError(TAG, "Two states have the same name in states list for soft button object");
79-
return;
75+
boolean repeatedStateNames = hasTwoStatesOfSameName(states);
76+
77+
boolean hasStateWithInitialName = false;
78+
for (SoftButtonState state : states) {
79+
if(state.getName().equals(initialStateName)) {
80+
hasStateWithInitialName = true;
81+
break;
82+
}
8083
}
8184

85+
if (repeatedStateNames) {
86+
DebugTool.logError(TAG, "A SoftButtonObject must have states with different names.");
87+
return;
88+
}
89+
if (!hasStateWithInitialName) {
90+
DebugTool.logError(TAG, "A SoftButtonObject must have a state with initialStateName.");
91+
return;
92+
}
8293
this.name = name;
8394
this.states = states;
8495
this.currentStateName = initialStateName;
@@ -264,6 +275,30 @@ public List<SoftButtonState> getStates() {
264275
* @param states a list of the object's soft button states. <strong>states should be unique for every SoftButtonObject. A SoftButtonState instance cannot be reused for multiple SoftButtonObjects.</strong>
265276
*/
266277
public void setStates(@NonNull List<SoftButtonState> states) {
278+
279+
boolean repeatedStateNames = hasTwoStatesOfSameName(states);
280+
281+
if (repeatedStateNames) {
282+
DebugTool.logError(TAG, "A SoftButtonObject must have states with different names.");
283+
return;
284+
}
285+
286+
if (states.isEmpty()) {
287+
DebugTool.logError(TAG, "A SoftButtonObject must contain at least one state");
288+
return;
289+
}
290+
291+
boolean hasStateWithCurrentName = false;
292+
for (SoftButtonState state : states) {
293+
if(state.getName().equals(currentStateName)) {
294+
hasStateWithCurrentName = true;
295+
break;
296+
}
297+
}
298+
if (!hasStateWithCurrentName) {
299+
DebugTool.logError(TAG, "A SoftButtonObject setting states must contain a state with the name " + currentStateName + ".");
300+
}
301+
267302
this.states = states;
268303
}
269304

0 commit comments

Comments
 (0)