Skip to content

On stopping auto mode in autopilot, the drive mode gets enable for phone controller i.e dual drive#436

Closed
hardikgarg02 wants to merge 9 commits into
ob-f:masterfrom
3dwesupport:android-vehicle-control
Closed

On stopping auto mode in autopilot, the drive mode gets enable for phone controller i.e dual drive#436
hardikgarg02 wants to merge 9 commits into
ob-f:masterfrom
3dwesupport:android-vehicle-control

Conversation

@hardikgarg02
Copy link
Copy Markdown
Contributor

No description provided.

@hardikgarg02 hardikgarg02 changed the title On stopping auto mode in vehicle control, the drive mode gets enable for phone controller i.e dual drive On stopping auto mode in autopilot, the drive mode gets enable for phone controller i.e dual drive Jun 25, 2024
@thias15
Copy link
Copy Markdown
Collaborator

thias15 commented Jul 4, 2024

Please resolve conflicts.


private void connectWebController() {
phoneController.connectWebServer();
Enums.DriveMode oldDriveMode = currentDriveMode;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I really don't like this code. But removing it means it won't remember the drive mode when we go back to game controller mode.

@thias15
Copy link
Copy Markdown
Collaborator

thias15 commented May 9, 2026

Closing this — apologies for the long delay before getting to it.

The guard that disables the drive-mode toggle while a phone/web controller is connected is the right idea, but the way this PR achieves it introduces a regression that outweighs the fix:

  1. Drive-mode preference is silently lost. Removing the oldDriveMode = currentDriveMode capture and the setDriveMode(oldDriveMode.getValue()) restore means once a user connects a phone/web controller, their original drive-mode preference is overwritten in SharedPreferences and never restored on disconnect. They're stuck on DUAL/GAME until they manually change it.
  2. CameraActivity.connectPhoneController now declares DriveMode oldDriveMode = currentDriveMode; but never reads it — dead variable, compiler warning.
  3. Guard logic is duplicated between AutopilotFragment.setNetworkEnabled and ObjectNavFragment.setNetworkEnabled (same 7-line block copy-pasted) — would drift over time.
  4. Conflicts with Unify the driveMode storage and fetching - from ControlsFragment.vehicle.driveMode, instead of ControlsFragment.currentDriveMode. #498 (which replaces currentDriveMode reads with vehicle.getDriveMode() in the same methods) and with current master.

If you'd like to revisit, a clean version would: keep the new guard in setNetworkEnabled (extracted to ControlsFragment so it isn't duplicated), and properly restore the user's prior drive mode in disconnectPhoneController instead of removing the save+restore entirely. Happy to review a fresh PR.

@thias15 thias15 closed this May 9, 2026
@github-project-automation github-project-automation Bot moved this from In progress to Done in OpenBot May 9, 2026
@thias15 thias15 deleted the android-vehicle-control branch May 9, 2026 04:41
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.

5 participants