Skip to content
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.

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

tools: add --auto flag to replay and cabana for loading routes from auto source #34863

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

deanlee
Copy link
Contributor

@deanlee deanlee commented Mar 14, 2025

Added the --auto flag to cabana and replay, allowing routes to be loaded from the most suitable source based on logreader.py's auto-sourcing logic.

Usage example for loading a specified route with auto-sourcing:

cabana --auto <route>
replay --auto <route>

@github-actions github-actions bot added the tools label Mar 14, 2025
@deanlee deanlee changed the title cabana: add --auto flag to load route from auto source using logreader.py cabana: add --auto flag to load route from auto source Mar 14, 2025
@deanlee deanlee changed the title cabana: add --auto flag to load route from auto source cabana: add --auto flag for route loading using logreader.py’s auto-sourcing logic Mar 14, 2025
@deanlee deanlee force-pushed the replay_auto_source branch from a8b7a54 to bc5fb7e Compare March 14, 2025 08:19
@deanlee deanlee changed the title cabana: add --auto flag for route loading using logreader.py’s auto-sourcing logic cabana: add --auto flag to load route from the most suitable source Mar 14, 2025
@deanlee deanlee mentioned this pull request Mar 14, 2025
2 tasks
@deanlee deanlee force-pushed the replay_auto_source branch from dbe20a9 to 6010ac1 Compare March 14, 2025 13:52
@deanlee deanlee changed the title cabana: add --auto flag to load route from the most suitable source tools: add --auto flag to replay and cabana for loading routes from auto source Mar 14, 2025
@deanlee deanlee force-pushed the replay_auto_source branch from 54f7a3d to 4bffe85 Compare March 14, 2025 19:57
@greatgitsby
Copy link
Contributor

could this instead be default behavior instead of behind a flag?

@sshane
Copy link
Contributor

sshane commented Mar 14, 2025

I agree, can we replace the C++ implementation of finding sources with this?

@deanlee
Copy link
Contributor Author

deanlee commented Mar 15, 2025

It's better to hide this feature behind a flag for now. While --auto offers a convenient option, it doesn’t replace the default route-loading behavior in replay&cabana.

The current auto-sourcing implementation has significant limitations. It only loads rlogs and fails to manage non-contiguous segments, which undermines its compatibility with key replay and Cabana features. These require smooth transitions between rlog/qlog, HEVC/qcam, and reliable handling of segment gaps.

Additionally, the auto-sourcing process is inefficient. It validates every source file with file_exists, triggering excessive HTTP calls that slow down startup—often taking more than 10 seconds on my laptop. I've also encountered frequent DNS errors, even when using a VPN.

The --auto flag should be removed once these issues and limitations are resolved.

@deanlee deanlee marked this pull request as ready for review March 15, 2025 04:25
@sshane
Copy link
Contributor

sshane commented Mar 15, 2025

Using /a handles missing rlogs in the middle of segments btw

@sshane
Copy link
Contributor

sshane commented Mar 15, 2025

Getting

  File "/home/batman/openpilot/tools/cabana/../lib/logreader.py", line 240, in auto_source
    raise LogsUnavailable("auto_source could not find any valid source, exceptions for sources:\n  - " +
LogsUnavailable: auto_source could not find any valid source, exceptions for sources:
  - internal_source: Exception('unable to get max_segment_number. ensure you have access to this route or the route is public.')
  - internal_source_zst: Exception('unable to get max_segment_number. ensure you have access to this route or the route is public.')
  - openpilotci_source: Exception('unable to get max_segment_number. ensure you have access to this route or the route is public.')
  - openpilotci_source_zst: Exception('unable to get max_segment_number. ensure you have access to this route or the route is public.')
  - comma_api_source: UnauthorizedError('Unauthorized. Authenticate with tools/lib/auth.py')
  - comma_car_segments_source: Exception('unable to get max_segment_number. ensure you have access to this route or the route is public.')
  - testing_closet_source: Exception('Internal source not available')

but I'm authenticated

@deanlee deanlee marked this pull request as draft March 15, 2025 05:44
@deanlee deanlee force-pushed the replay_auto_source branch 2 times, most recently from c89a391 to bb93f9a Compare March 15, 2025 12:53
@deanlee
Copy link
Contributor Author

deanlee commented Mar 15, 2025

@sshane fixed. cabana use OPENPILOT_PREFIX to run multiple instances simultaneously. This was causing the path to auth.json to be prefixed, making it unreadable.

@deanlee deanlee force-pushed the replay_auto_source branch from 79ff2dd to 9e6c5a3 Compare March 15, 2025 17:38
@deanlee deanlee marked this pull request as ready for review March 15, 2025 17:47
@deanlee deanlee force-pushed the replay_auto_source branch from 9e6c5a3 to d9899e7 Compare March 16, 2025 16:51
@deanlee deanlee marked this pull request as draft March 16, 2025 16:53
@deanlee deanlee force-pushed the replay_auto_source branch from d9899e7 to 83f0e90 Compare March 16, 2025 18:56
@deanlee deanlee marked this pull request as ready for review March 16, 2025 19:42
@deanlee deanlee force-pushed the replay_auto_source branch from edeaff8 to cdfa9ec Compare March 18, 2025 14:52
@sshane
Copy link
Contributor

sshane commented Mar 24, 2025

Tried it with a route from test_models:

batman@workstation-shane:~/openpilot$ cabana 0c94aa1e1296d7c6/2021-05-05--19-48-37/2 --auto
active services: can, carParams, driverEncodeIdx, roadEncodeIdx, wideRoadEncodeIdx
loading route 
terminate called after throwing an instance of 'std::out_of_range'
  what():  basic_string::substr: __pos (which is 5) > this->size() (which is 4)
Aborted (core dumped)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants