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

[RSDK-3786] full-mod-new-close #186

Merged

Conversation

nicksanford
Copy link
Collaborator

@nicksanford nicksanford commented Jul 7, 2023

Ticket

  1. Make viamcartographer.New initialize 1.) a carto facade instance 2.) a background go routine which will call into the carto facade C code 3.) a sensor process go routine to add sensor data when the full mod feature flag is enabled
  2. Make viamcartographer.Close terminate in the proper order the resources initialized by viamcartographer.New
  3. Add algo config defaults & parsing logic (pre full mod this was a C++ code responsibility, now it is a go code responsibility)
  4. Don't setup deprecated directory structure if full mod feature flag is enabled, as otherwise it always causes full mod carto to throw errors on initialization (as per the project requirements)
  5. Change package structure to fix circular dependency issues which would otherwise be caused by making viam-cartographer dependent on the sensorprocess package.
  6. Change Makefile's build target to compile the C++ code (if not already compiled) before compiling the go code. This is needed as now main.go needs to link in C code, which was not the case prior. W/o this change, it doesn't build.

@viambot viambot added the safe to test This pull request is marked safe to test from a trusted zone label Jul 7, 2023
@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Jul 7, 2023
@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Jul 7, 2023
@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Jul 7, 2023
@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Jul 7, 2023
@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Jul 7, 2023
viam-cartographer.go Outdated Show resolved Hide resolved
viam-cartographer.go Outdated Show resolved Hide resolved
viam-cartographer.go Outdated Show resolved Hide resolved
@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Jul 10, 2023
@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Jul 10, 2023
nicksanford and others added 2 commits July 11, 2023 12:17
Co-authored-by: kim-mishra <121991867+kim-mishra@users.noreply.github.com>
Co-authored-by: kim-mishra <121991867+kim-mishra@users.noreply.github.com>
@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Jul 11, 2023
Co-authored-by: kim-mishra <121991867+kim-mishra@users.noreply.github.com>
@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Jul 11, 2023
Co-authored-by: kim-mishra <121991867+kim-mishra@users.noreply.github.com>
@viambot viambot removed the safe to test This pull request is marked safe to test from a trusted zone label Jul 11, 2023
Co-authored-by: kim-mishra <121991867+kim-mishra@users.noreply.github.com>
@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Jul 11, 2023
viam-cartographer.go Outdated Show resolved Hide resolved
Co-authored-by: kim-mishra <121991867+kim-mishra@users.noreply.github.com>
@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Jul 11, 2023
@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Jul 11, 2023
Copy link
Collaborator

@kim-mishra kim-mishra left a comment

Choose a reason for hiding this comment

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

LGTM!

@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Jul 11, 2023
@nicksanford nicksanford merged commit b381453 into viam-modules:main Jul 11, 2023
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
safe to test This pull request is marked safe to test from a trusted zone
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants