LCAS / hunter_robot

Apache License 2.0
0 stars 0 forks source link

refactor hunter_robot repo #3

Closed marc-hanheide closed 4 months ago

marc-hanheide commented 5 months ago

This contains copied code at the moment. It shouldn't be copied. 2 Options for @ibrahimhroob to consider:

  1. fork the original repository https://github.com/agilexrobotics/hunter_ros2 and make our own additions (This is if we need to make changes to it)
  2. include the original repository as submodule (if we can leave the original sources untouched)

A fork allows to pull in changes and bug fixes from upstream.

Sorry, @PabloEspejeLS, one must never copy source code ;-)

ibrahimhroob commented 5 months ago

This issue has been resolved in pull request #4. I opted for the submodule approach, but instead of incorporating the original repository, I included a submodule of a forked repository under LCAS ws.

ibrahimhroob commented 5 months ago

Hi @marc-hanheide, git submodule may cause an issue when cloning the dependencies when building the devcontainer, so there are two options: either try git subtree, or use the fork version, but for the latter I need to copy the developed packages into the forked repo.

marc-hanheide commented 5 months ago

Thanks @ibrahimhroob ,

I'm not convinced this is the right way. submodules are an issue when we want to release properly from a repository. As by ROS convention all packages in a repository must have the same <version> in their package.xml which is a problem to maintain if they are in different repos (submodules).

I have just now triggered a proper release of the ugv_sdk package (forked into https://github.com/LCAS/ugv_sdk). This means we should afterwards be able to simply apt-get install this as ros-humble-ugv-sdk from our jammy repository. So, that that dependency is already properly dealt with. (It's still building, so doesn't exist yet).

Once that is done, I think we should do the same with the LCAS/hunter_ros2 repository, i.e., release it properly, and then NOT have a submodule at all, but just declare the dependency in package.xml as needed for hunter_robot.

This is always the preferred way, I would say.

marc-hanheide commented 5 months ago

I have worked on releasing the depending packages.

See https://github.com/LCAS/rosdistro/pull/581 and https://github.com/LCAS/rosdistro/pull/580

So, with this the dependencies should be available and we shouldn't need the submodules at all, just the bits we really need in this repo should be here, and dependencies correctly declared.

marc-hanheide commented 5 months ago

So, can I suggest you do the following: