Skip to content

Commit 6622d77

Browse files
committed
Standardize controllers (#59)
This commit standardizes the MachineController to make it has the union of its children's methods. We keep MachineController as it were and add 'CarControllerBase' for car-formed machines such as the car form of Nxt / Ev3 and Pile robots, and rename NxtController after it (NxtCarController). By this change, the previous block classes and resources related to the car-formed machines can be moved in 'main' variant and we decide to keep them in 'car' package for readability (StopForSecBlock is not moved because it doesn't depend on the car-formed machines' method). This commit also cleans string resources. * TL;DR ** Background We talked over the design of controllers again and again offline, and we had two options; - Make controllers for each machine (Nxt, Ev3, Pile, etc.) so that they have their specific methods (e.g., Pile Robot doesn't have ColorSensor, thus it doesn't have getColorSensorValue or something like that) and keep the MachineController as it is. - Make the common MachineController that has the union of methods of these controllers. They actually have pros & cons and our opinions conflicted. But finally, at present, we decided to choose the latter. ** Pros & Cons of the former *** Pros - Straightforward way and thus readable. This is very important for developers. - They can be easily tested. *** Cons - There will be many duplications of codes because these controllers use [our library](https://github.com/PileProject/drivecommand)'s MachineBase that provides the same interface to control machines (so, we will have very similar codes in blocks like [ForwardSecBlock](https://github.com/PileProject/drive/blob/develop/app/src/nxt/java/com/pileproject/drive/programming/visual/block/sequence/ForwardSecBlock.java) in each build variant) but we currently can't make the most of it because of the design. - It doesn't follow the design of [our library](https://github.com/PileProject/drivecommand). But we can fix that of our library easily. ** Pros & Cons of the latter *** Pros - It follows the design of [our library](https://github.com/PileProject/drivecommand). - It commonizes the blocks through MachineController interface. Thus, it reduces the duplicated codes not only *.java but also *.xml. - Easy to maintain because the codes are unified. *** Cons - It is difficult to test each variant. - Not straightforward way. - Users can't know what kinds of methods each controller has, and make it worse, calling wrong methods lead to RuntimeError not CompileError. - Poor extensibility. We should add/remove a method to/from MachineController itself if new machines with new methods. So, maybe we should make an another layer for other machines something like MachineController -> CarController, DroneController (i.e., not use the MachineController as the current NxtController's parent). ** Why we chose the latter This was a very difficult problem for us but we took the pros of the latter. Maintaining duplicated codes *.java and *.xml is very annoying and we can easily change our mind when a new type of machine comes if we keep the unified codes clean.
1 parent 9b9dcdc commit 6622d77

File tree

79 files changed

+968
-702
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

79 files changed

+968
-702
lines changed

.idea/copyright/pileproject.xml

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

.idea/gradle.xml

Lines changed: 1 addition & 6 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

app/src/androidTest/java/com/pileproject/drive/execution/NxtControllerTest.java renamed to app/src/androidTest/java/com/pileproject/drive/execution/NxtCarControllerTest.java

Lines changed: 21 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@
1717

1818
import android.support.test.runner.AndroidJUnit4;
1919

20+
import com.pileproject.drive.machine.CarControllerBase;
21+
import com.pileproject.drive.machine.NxtCarController;
2022
import com.pileproject.drivecommand.machine.device.input.LineSensor;
2123
import com.pileproject.drivecommand.machine.device.input.SoundSensor;
2224
import com.pileproject.drivecommand.machine.device.input.TouchSensor;
@@ -30,6 +32,8 @@
3032
import org.mockito.Mock;
3133
import org.mockito.internal.util.reflection.Whitebox;
3234

35+
import static com.pileproject.drive.machine.CarControllerBase.MotorKind.LeftMotor;
36+
import static com.pileproject.drive.machine.CarControllerBase.MotorKind.RightMotor;
3337
import static junit.framework.Assert.assertEquals;
3438
import static junit.framework.Assert.assertFalse;
3539
import static junit.framework.Assert.assertTrue;
@@ -38,10 +42,11 @@
3842
import static org.mockito.Mockito.verify;
3943

4044
@RunWith(AndroidJUnit4.class)
41-
public class NxtControllerTest {
45+
public class NxtCarControllerTest {
4246

4347
@Mock NxtMachine machine;
44-
@InjectMocks NxtController controller;
48+
@InjectMocks
49+
NxtCarController controller;
4550

4651
@Mock TouchSensor touchSensor;
4752
@Mock SoundSensor soundSensor;
@@ -53,7 +58,7 @@ public class NxtControllerTest {
5358
@Before
5459
public void setUp() {
5560
machine = mock(NxtMachine.class);
56-
controller = new NxtController(machine);
61+
controller = new NxtCarController(machine);
5762

5863
touchSensor = mock(TouchSensor.class);
5964
soundSensor = mock(SoundSensor.class);
@@ -76,38 +81,38 @@ private void setUpMotors() {
7681
public void whenTouchSensorIsNull_thenReturnFalse() throws Exception {
7782
Whitebox.setInternalState(controller, "mTouchSensor", null);
7883

79-
assertFalse(controller.getTouchSensorValue());
84+
assertFalse(controller.isTouchSensorTouched());
8085
}
8186

8287
@Test
8388
public void whenTouchSensorIsNotNull_andTouchSensorIsTouched_thenReturnTrue() throws Exception {
8489
Whitebox.setInternalState(controller, "mTouchSensor", touchSensor);
8590
doReturn(true).when(touchSensor).isTouched();
8691

87-
assertTrue(controller.getTouchSensorValue());
92+
assertTrue(controller.isTouchSensorTouched());
8893
}
8994

9095
@Test
9196
public void whenTouchSensorIsNotNull_andTouchSensorIsNotTouched_thenReturnFalse() throws Exception {
9297
Whitebox.setInternalState(controller, "mTouchSensor", touchSensor);
9398
doReturn(false).when(touchSensor).isTouched();
9499

95-
assertFalse(controller.getTouchSensorValue());
100+
assertFalse(controller.isTouchSensorTouched());
96101
}
97102

98103
@Test
99104
public void whenSoundSensorIsNull_thenReturnNegative() throws Exception {
100105
Whitebox.setInternalState(controller, "mSoundSensor", null);
101106

102-
assertEquals(controller.getSoundSensorValue(), -1);
107+
assertEquals(controller.getSoundSensorDb(), -1);
103108
}
104109

105110
@Test
106111
public void whenSoundSensorIsNotNull_thenReturnProperValue() throws Exception {
107112
Whitebox.setInternalState(controller, "mSoundSensor", soundSensor);
108113
doReturn(10).when(soundSensor).getDb();
109114

110-
assertEquals(10, controller.getSoundSensorValue());
115+
assertEquals(10, controller.getSoundSensorDb());
111116
}
112117

113118
@Test
@@ -181,16 +186,16 @@ public void whenMotorSpeedsAreNotInitialized_thenMovesForwardWithDefaultValue()
181186

182187
controller.moveForward();
183188

184-
verify(leftMotor).setSpeed(NxtController.INIT_MOTOR_POWER);
185-
verify(rightMotor).setSpeed(NxtController.INIT_MOTOR_POWER);
189+
verify(leftMotor).setSpeed(CarControllerBase.MotorProperty.INIT_MOTOR_POWER);
190+
verify(rightMotor).setSpeed(CarControllerBase.MotorProperty.INIT_MOTOR_POWER);
186191
}
187192

188193
@Test
189194
public void whenSetMotorPowerCalled_thenMovesForwardWithTheValue() throws Exception {
190195
setUpMotors();
191196

192-
controller.setMotorPower(NxtController.MotorKind.LeftMotor, 10);
193-
controller.setMotorPower(NxtController.MotorKind.RightMotor, 10);
197+
controller.setMotorPower(LeftMotor, 10);
198+
controller.setMotorPower(RightMotor, 10);
194199

195200
controller.moveForward();
196201

@@ -202,8 +207,8 @@ public void whenSetMotorPowerCalled_thenMovesForwardWithTheValue() throws Except
202207
public void whenSetMotorPowerCalledWithOverUpperBoundValue_thenMovesForwardWithUpperBoundValue() throws Exception {
203208
setUpMotors();
204209

205-
controller.setMotorPower(NxtController.MotorKind.LeftMotor, 200);
206-
controller.setMotorPower(NxtController.MotorKind.RightMotor, 200);
210+
controller.setMotorPower(LeftMotor, 200);
211+
controller.setMotorPower(RightMotor, 200);
207212

208213
controller.moveForward();
209214

@@ -215,8 +220,8 @@ public void whenSetMotorPowerCalledWithOverUpperBoundValue_thenMovesForwardWithU
215220
public void whenSetMotorPowerCalledWithUnderLowerBoundValue_thenMovesForwardWithLowerBoundValue() throws Exception {
216221
setUpMotors();
217222

218-
controller.setMotorPower(NxtController.MotorKind.LeftMotor, -10);
219-
controller.setMotorPower(NxtController.MotorKind.RightMotor, -10);
223+
controller.setMotorPower(LeftMotor, -10);
224+
controller.setMotorPower(RightMotor, -10);
220225

221226
controller.moveForward();
222227

app/src/main/java/com/pileproject/drive/database/DriveDatabase.java

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,7 @@
2424
import com.yahoo.squidb.sql.Table;
2525

2626
/**
27-
* Implementation of SquidDatabase for this app.
28-
*
29-
* @author <a href="mailto:tatsuyaw0c@gmail.com">Tatsuya Iwanari</a>
30-
* @version 1.0 3-April-2016
27+
* The implementation of SquidDatabase for this app.
3128
*/
3229
public class DriveDatabase extends SquidDatabase {
3330
private static final int VERSION = 1;

app/src/main/java/com/pileproject/drive/database/ProgramDataManager.java

Lines changed: 27 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -30,14 +30,11 @@
3030
import java.util.Collections;
3131

3232
/**
33-
* Program Data manager
34-
*
35-
* @author <a href="mailto:tatsuyaw0c@gmail.com">Tatsuya Iwanari</a>
36-
* @version 2.0 2-April-2016
33+
* A manger of {@link ProgramData}.
3734
*/
3835
public class ProgramDataManager {
3936
private static ProgramDataManager mInstance = new ProgramDataManager();
40-
private DriveDatabase mDriveDatabase = null;
37+
private DriveDatabase mDriveDatabase;
4138

4239
// the number of execution programs should be less than or equals to 1
4340
private static final Query EXECUTION_PROGRAM =
@@ -71,29 +68,29 @@ public static ProgramDataManager getInstance() {
7168
}
7269

7370
/**
74-
* Save program data temporarily to execute it
71+
* Saves a program temporarily to execute it.
7572
*
76-
* @param BlockSpaceLayout The programming space that has blocks
73+
* @param layout The programming space that has blocks
7774
*/
7875
public boolean saveExecutionProgram(BlockSpaceLayout layout) {
7976
return saveProgram(Program.EXECUTION, Program.EXECUTION, layout);
8077
}
8178

8279
/**
83-
* Save a sample program data
80+
* Saves a sample program.
8481
*
85-
* @param String the name of a new program
86-
* @param BlockSpaceLayout the programming space that has blocks
82+
* @param programName the name of a new program
83+
* @param layout the programming space that has blocks
8784
*/
8885
public boolean saveSampleProgram(String programName, BlockSpaceLayout layout) {
8986
return saveProgram(programName, Program.SAMPLE, layout);
9087
}
9188

9289
/**
93-
* Save a user program data
90+
* Saves a user program.
9491
*
95-
* @param String the name of a new program
96-
* @param BlockSpaceLayout the programming space that has blocks
92+
* @param programName the name of a new program
93+
* @param layout the programming space that has blocks
9794
*/
9895
public boolean saveUserProgram(String programName, BlockSpaceLayout layout) {
9996
return saveProgram(programName, Program.USER, layout);
@@ -141,29 +138,29 @@ private boolean saveProgram(String programName, String programType, BlockSpaceLa
141138
}
142139

143140
/**
144-
* Load an execution program's block data (Sorted)
141+
* Loads an execution program's block data (sorted).
145142
*
146-
* @return ArrayList<BlockBase> loaded data
143+
* @return loaded data as {@link ArrayList<BlockBase>}
147144
*/
148145
public ArrayList<BlockBase> loadExecutionProgram() {
149146
return loadProgram(Program.EXECUTION, Program.EXECUTION);
150147
}
151148

152149
/**
153-
* Load a sample program's block data (Sorted)
150+
* Loads a sample program's block data (sorted).
154151
*
155-
* @param String the name of program
156-
* @return ArrayList<BlockBase> loaded data
152+
* @param programName the name of program
153+
* @return loaded data as {@link ArrayList<BlockBase>}
157154
*/
158155
public ArrayList<BlockBase> loadSampleProgram(String programName) {
159156
return loadProgram(programName, Program.SAMPLE);
160157
}
161158

162159
/**
163-
* Load a user program's block data
160+
* Loads a user program's block data
164161
*
165-
* @param String the name of program
166-
* @return ArrayList<BlockBase> loaded data
162+
* @param programName the name of program
163+
* @return loaded data as {@link ArrayList<BlockBase>}
167164
*/
168165
public ArrayList<BlockBase> loadUserProgram(String programName) {
169166
return loadProgram(programName, Program.USER);
@@ -217,18 +214,18 @@ private ArrayList<BlockBase> loadBlocks(SquidCursor<ProgramData> c) {
217214
}
218215

219216
/**
220-
* Load all sample program names
217+
* Loads all the sample program names.
221218
*
222-
* @return ArrayList<String> the names of sample programs
219+
* @return the names of sample programs as {@link ArrayList<String>}
223220
*/
224221
public ArrayList<String> loadSampleProgramNames() {
225222
return loadProgramNames(Program.SAMPLE);
226223
}
227224

228225
/**
229-
* Load all user program names
226+
* Loads all user program names
230227
*
231-
* @return ArrayList<String> the names of user programs
228+
* @return the names of user programs as {@link ArrayList<String>}
232229
*/
233230
public ArrayList<String> loadUserProgramNames() {
234231
return loadProgramNames(Program.USER);
@@ -256,24 +253,24 @@ else if (programType.equals(Program.SAMPLE)) {
256253
}
257254

258255
/**
259-
* Delete an execution program
256+
* Deletes an execution program.
260257
*/
261258
public void deleteExecutionProgram() {
262259
deleteProgram(Program.EXECUTION, Program.EXECUTION);
263260
}
264261

265262
/**
266-
* Delete a sample program with 'programName'
263+
* Deletes a sample program with <code>programName</code>.
267264
*
268-
* @param String the name of a sample program
265+
* @param programName the name of a sample program
269266
*/
270267
public void deleteSampleProgram(String programName) {
271268
deleteProgram(programName, Program.SAMPLE);
272269
}
273270
/**
274-
* Delete a user program with 'programName'
271+
* Deletes a user program with <code>programName</code>.
275272
*
276-
* @param String the name of a user program
273+
* @param programName the name of a user program
277274
*/
278275
public void deleteUserProgram(String programName) {
279276
deleteProgram(programName, Program.USER);

app/src/main/java/com/pileproject/drive/database/ProgramDataSpec.java

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,6 @@
2121

2222
/**
2323
* Specification for "program_data" table
24-
*
25-
* @author <a href="mailto:tatsuyaw0c@gmail.com">Tatsuya Iwanari</a>
26-
* @version 1.0 22-March-2016
2724
*/
2825
@TableModelSpec(className="ProgramData", tableName="program_data",
2926
tableConstraint = "FOREIGN KEY(programId) references programs(_id) ON DELETE CASCADE")

app/src/main/java/com/pileproject/drive/database/ProgramSpec.java

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,6 @@
2222

2323
/**
2424
* Specification for "programs" table
25-
*
26-
* @author <a href="mailto:tatsuyaw0c@gmail.com">Tatsuya Iwanari</a>
27-
* @version 1.0 22-March-2016
2825
*/
2926
@TableModelSpec(className="Program", tableName="programs")
3027
public class ProgramSpec {

app/src/main/java/com/pileproject/drive/execution/ExecutionActivity.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
import com.pileproject.drive.comm.CommunicatorProvider;
3636
import com.pileproject.drive.comm.RxMachineConnector;
3737
import com.pileproject.drive.database.ProgramDataManager;
38+
import com.pileproject.drive.machine.MachineProvider;
3839
import com.pileproject.drive.util.bluetooth.BluetoothUtil;
3940
import com.pileproject.drive.util.development.DeployUtils;
4041
import com.pileproject.drive.util.fragment.AlertDialogFragment;

app/src/main/java/com/pileproject/drive/execution/RxObservableProgram.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717

1818
import android.os.Bundle;
1919

20+
import com.pileproject.drive.machine.MachineController;
2021
import com.pileproject.drive.programming.visual.block.BlockBase;
2122

2223
import java.util.List;

0 commit comments

Comments
 (0)