autoware icon indicating copy to clipboard operation
autoware copied to clipboard

Move drivers into separate .repos file

Open xmfcx opened this issue 2 years ago • 6 comments

Checklist

  • [X] I've read the contribution guidelines.
  • [X] I've searched other issues and no duplicate issues were found.
  • [X] I've agreed with the maintainers that I can plan this task.

Description

This issue is about moving the driver packages to a separate driver.repos file from autoware.repos file.

Purpose

Since not every vehicle will use all the drivers, it's better for them to have a separate driver.repos file.

This way when people fork autoware, it's easier for them to disable these repositories.

Possible approaches

We should simply separate the driver repositories.

But we will probably need to update the CI and the documentation for installation.

Current drivers:

  sensor_component/external/tamagawa_imu_driver:
    type: git
    url: https://github.com/tier4/tamagawa_imu_driver.git
    version: ros2
  sensor_component/external/velodyne_vls:
    type: git
    url: https://github.com/tier4/velodyne_vls.git
    version: tier4/universe
  vehicle/external/pacmod_interface:
    type: git
    url: https://github.com/tier4/pacmod_interface.git
    version: main

Definition of done

  • [ ] pacmod_interface is moved out
  • [ ] velodyne_vls is moved out
  • [ ] tamagawa_imu_driver is moved out
  • [ ] Documentations are updated
  • [ ] CI is updated

xmfcx avatar Mar 24 '23 12:03 xmfcx

@kenji-miyake @mitsudome-r @esteve maybe we can also discuss about whether we should build these packages as part of the CI or not.

Or discuss whether we should do the proposal on this issue at all.

From these 3, I think velodyne is the more essential one but I'm not sure about what to do with the others. Any thought is appreciated.

xmfcx avatar Mar 24 '23 12:03 xmfcx

@xmfcx I think pacmod could be moved out easily, but as you say Velodyne is essential to our architecture, so I'm not sure if it should be moved out. The Tamagawa IMU driver doesn't seem to be used much, though. Only two packages depend on it: sample_sensor_kit_launch and awsim_sensor_kit_launch

esteve avatar Mar 24 '23 12:03 esteve

@xmfcx I'm sorry for joining late. Explaining the background, these are from TIER IV's reference vehicle/sensor_kit.

I think splitting repos or just removing some is okay, but there are several things to be discussed:

  1. Which perspective to divide

Do you really want to split drivers? Or do you want to split the entire sensor components? I'm not sure which is better.

Since not every vehicle will use all the drivers, it's better for them to have a separate driver.repos file.

Seeing this purpose, I feel like you want to split the sensor components that aren't related to your vehicle.

This way when people fork autoware, it's easier for them to disable these repositories.

For this, I think users can do it with the current configuration because they are under the sensor_component section. :thinking: Could you please show me some concrete examples?

  1. Dependency

We might need to fix some dependencies before splitting them out.


Anyway, I'd like to clarify the background and issues more. Honestly, I feel that the current proposal might be a bit short-sighted. So could you share your troubles in more detail with me?

kenji-miyake avatar Apr 11 '23 01:04 kenji-miyake

@xmfcx Could you share more detail about the purpose of this issue?

  • Is it to create new repos file to list up Autoware-supported sensor drivers?, or
  • Do you just want to remove the need of installing unnecessary drivers?

If latter is the case, I think we can just remove Tamagawa-imu and pacmod-interface drivers since they are not used in tutorials anyways.

mitsudome-r avatar Apr 12 '23 08:04 mitsudome-r

So, I have thought about this more generally and here are the things I'd like to achieve:

  • Users shouldn't be forced to build drivers that they won't use.
  • The drivers should be tested in the repository through the CI to keep compatible.
  • Some drivers are essential, like the nebula driver.

For this, I will separate out the following:

  • pacmod_interface
  • tamagawa_imu_driver

into a different .repos file called extra-packages.repos.

Will modify the CI to pull them along with the others, build and test as usual.

The users will download the usual autoware.repos without these packages.

If they want to, they can download it with the extra-packages.repos.

In the future, we can think of similar non-essential external packages to this list.

Do you think this is reasonable? @esteve @mitsudome-r ?

xmfcx avatar Nov 16 '23 13:11 xmfcx

@xmfcx I agree with you, the drivers are quite specific to the vehicle where Autoware is running, so it'd be better to move them to a separate .repos file and let users use whatever driver they need for their vehicle.

esteve avatar Nov 16 '23 14:11 esteve