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-6040 Add GetProperties endpoint to viam-cartographer #310

Merged
merged 9 commits into from
Jan 10, 2024

Conversation

jeremyrhyde
Copy link
Contributor

@jeremyrhyde jeremyrhyde commented Dec 26, 2023

This PR implements Properties for viam-cartographer.

Related PR: viamrobotics/rdk#3376

JIRA Ticket: https://viam.atlassian.net/browse/RSDK-6040

@viambot viambot added the safe to test This pull request is marked safe to test from a trusted zone label Dec 26, 2023
@jeremyrhyde jeremyrhyde marked this pull request as ready for review December 26, 2023 18:22
@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 Dec 26, 2023
go.mod Outdated
@@ -329,3 +329,5 @@ require (
periph.io/x/conn/v3 v3.7.0 // indirect
periph.io/x/host/v3 v3.8.2 // indirect
)

replace go.viam.com/rdk => /Users/jeremyhyde/Development/rdk
Copy link
Contributor Author

Choose a reason for hiding this comment

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

[note] to be removed after viamrobotics/rdk#3376 and new rdk release

@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 Dec 26, 2023
@@ -678,3 +689,17 @@ func CheckQuaternionFromClientAlgo(pose spatialmath.Pose, componentReference str
}
return nil, "", errors.Errorf("error getting SLAM position: quaternion not given, %v", returnedExt)
}

// checkCloseAndCloudStatus returns an error if the cartographer service has been previously closed or is being run in the cloud.
func (cartoSvc *CartographerService) checkCloseAndCloudStatus(cmd string) error {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
func (cartoSvc *CartographerService) checkCloseAndCloudStatus(cmd string) error {
func (cartoSvc *CartographerService) isOpenAndRunningLocally(cmd string) error {

Copy link
Collaborator

Choose a reason for hiding this comment

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

[nit] think a name starting with is might read a little cleaner in all of the functions it is used in

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

viam_cartographer.go Outdated Show resolved Hide resolved
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.

2 nits otherwise lgtm

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 Dec 27, 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 Dec 27, 2023
@jeremyrhyde jeremyrhyde changed the title RSDK-6040 Add Properties endpoint RSDK-6040 Add Properties Endpoint to viam-cartographer Dec 29, 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 Dec 29, 2023
@jeremyrhyde jeremyrhyde changed the title RSDK-6040 Add Properties Endpoint to viam-cartographer RSDK-6040 Add GetProperties endpoint to viam-cartographer Jan 2, 2024
@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 Jan 10, 2024
@viambot viambot removed the safe to test This pull request is marked safe to test from a trusted zone label Jan 10, 2024
@viambot viambot added the safe to test This pull request is marked safe to test from a trusted zone label Jan 10, 2024
@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 Jan 10, 2024
@jeremyrhyde jeremyrhyde merged commit c318e49 into viam-modules:main Jan 10, 2024
6 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.

3 participants