Unify the driveMode storage and fetching - from ControlsFragment.vehicle.driveMode, instead of ControlsFragment.currentDriveMode.#498
Conversation
…stead of ControlsFragment.currentDriveMode
There was a problem hiding this comment.
Pull Request Overview
This PR refactors drive mode state management by eliminating the redundant currentDriveMode field and replacing it with direct calls to vehicle.getDriveMode(). This change improves maintainability by ensuring the vehicle object is the single source of truth for drive mode state.
- Removes the
currentDriveModefield fromControlsFragmentbase class - Updates all references to use
vehicle.getDriveMode()instead - Affects multiple fragment implementations: FreeRoamFragment, ObjectNavFragment, LoggerFragment, and AutopilotFragment
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| android/robot/src/main/java/org/openbot/common/ControlsFragment.java | Removes the currentDriveMode field declaration and updates status reporting to use vehicle.getDriveMode() |
| android/robot/src/main/java/org/openbot/robot/FreeRoamFragment.java | Replaces currentDriveMode references with vehicle.getDriveMode() in controller connection methods |
| android/robot/src/main/java/org/openbot/objectNav/ObjectNavFragment.java | Replaces currentDriveMode references with vehicle.getDriveMode() in controller connection methods |
| android/robot/src/main/java/org/openbot/logging/LoggerFragment.java | Replaces currentDriveMode references with vehicle.getDriveMode() in controller connection methods |
| android/robot/src/main/java/org/openbot/autopilot/AutopilotFragment.java | Replaces currentDriveMode references with vehicle.getDriveMode() in controller connection methods |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Enums.DriveMode oldDriveMode = vehicle.getDriveMode(); | ||
| // Currently only dual drive mode supported | ||
| setDriveMode(Enums.DriveMode.GAME); |
There was a problem hiding this comment.
Inconsistent type reference. This file uses a static import for DriveMode (line 4), so Enums.DriveMode should be simplified to DriveMode for consistency. Line 265 and 267 use DriveMode, but this line uses Enums.DriveMode. Please use DriveMode consistently throughout the method.
| Enums.DriveMode oldDriveMode = vehicle.getDriveMode(); | |
| // Currently only dual drive mode supported | |
| setDriveMode(Enums.DriveMode.GAME); | |
| DriveMode oldDriveMode = vehicle.getDriveMode(); | |
| // Currently only dual drive mode supported | |
| setDriveMode(DriveMode.GAME); |
| Enums.DriveMode oldDriveMode = currentDriveMode; | ||
| Enums.DriveMode oldDriveMode = vehicle.getDriveMode(); | ||
| // Currently only dual drive mode supported | ||
| setDriveMode(Enums.DriveMode.GAME); |
There was a problem hiding this comment.
Inconsistent type reference. This file uses a static import for DriveMode (line 4), so Enums.DriveMode.GAME should be simplified to DriveMode.GAME for consistency with line 267 which uses DriveMode.DUAL.
| setDriveMode(Enums.DriveMode.GAME); | |
| setDriveMode(DriveMode.GAME); |
Unify the driveMode storage and fetching - from ControlsFragment.vehicle.driveMode, instead of ControlsFragment.currentDriveMode.
There are two places containing drive mode: ControlsFragment.currentDriveMode, and ControlsFragment.vehicle.driveMode. They should be unified. ControlsFragment.vehicle.driveMode is used more widely(as shown below) and is supposed to be the final one.