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

AFTER COMPETITION: Join CV + ROS repos #113

Open
3 tasks
thedavidchu opened this issue Jun 1, 2022 · 3 comments
Open
3 tasks

AFTER COMPETITION: Join CV + ROS repos #113

thedavidchu opened this issue Jun 1, 2022 · 3 comments
Labels
enhancement New feature or request question Further information is requested

Comments

@thedavidchu
Copy link
Contributor

Description:

  • Needing to port CV code from the CV-Pipeline repo to the Caffeine repo during CV/ROS integration is a real pain.
  • KJ is right: CV should be subordinate to ROS
  • This issue is just kind of a suggestion, built off of KJ's observation and mixed with a few ideas for more structural clean code/etc

Goals:

  • Copy over all CV (clearly mark what is Python 3 vs Python 2... using (shebang)[https://en.wikipedia.org/wiki/Shebang_(Unix)] operators would help)
  • Clean up code + unify styles (maybe setup a linter (Black for Python, clang-format for C++???)/ style guide (e.g. "Defer to ___"))
  • Set up ci-testing framework with github actions. Require people to write their own tests (maybe???). This is much easier said than done

Notes:

  • Very much an "after competition" task
  • Very much a suggestion and open to discussion!
@spencerteetaert
Copy link
Contributor

We haven't let ourselves consider this too much because this repo will likely end after competition. Upgrades are in place for ros and cmake versions that will render a lot of this code useless. Will be a question for next year's team

@kajananchinniah
Copy link
Collaborator

Just to contribute the conversation a bit:

This repo will be inactive after this year if next year uses ROS2. If they use ROS1, it can serve as a reference.

In the future, we should have CV live in the ROS repo, as discussed offline since we ultimately need to have a fork of the CV code here.

The python2 vs python3 issue shouldn't be present next year whether they choose ROS2 or ROS1, assuming they use noetic / 20.04.

Having a style guide would be useful, black & clang-format would be my go-to too, and is low hanging fruit so it would be good to have a GitHub action that does it.

However, more importantly having proper dependency / environment management (e.g. via Docker) is a higher priority imo. Robosoccer has a ARM + x86_64 based build so would be useful to do something similar.

CI I won't bother with, unless we rethink how this repo handles nodes & have people willing to teach new people how to handle it. One issue we have is a lot of testing is done in sim.

@kajananchinniah
Copy link
Collaborator

kajananchinniah commented Jun 2, 2022

Granted, setting up CI to just sanity check the build was something I was thinking about.

I also think the cv integration should be happening while CV is in development rather than after.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request question Further information is requested
Projects
None yet
Development

No branches or pull requests

9 participants