Skip to content

Commit

Permalink
remove component reference from cartofacade
Browse files Browse the repository at this point in the history
  • Loading branch information
JohnN193 committed May 22, 2024
1 parent af97519 commit bc0e97d
Show file tree
Hide file tree
Showing 6 changed files with 11 additions and 49 deletions.
11 changes: 3 additions & 8 deletions cartofacade/capi.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,6 @@ type Position struct {
Imag float64
Jmag float64
Kmag float64

ComponentReference string
}

// LidarConfig represents the lidar configuration
Expand All @@ -93,10 +91,9 @@ const (

// CartoConfig contains config values from app
type CartoConfig struct {
Camera string
MovementSensor string
ComponentReference string
LidarConfig LidarConfig
Camera string
MovementSensor string
LidarConfig LidarConfig

EnableMapping bool
ExistingMap string
Expand Down Expand Up @@ -440,8 +437,6 @@ func toPositionResponse(value C.viam_carto_get_position_response) Position {
Imag: float64(value.imag),
Jmag: float64(value.jmag),
Kmag: float64(value.kmag),

ComponentReference: bstringToGoString(value.component_reference),
}
}

Expand Down
4 changes: 0 additions & 4 deletions cartofacade/capi_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,6 @@ func TestPositionResponse(t *testing.T) {
gpr := getTestPositionResponse()
t.Run("position response properly converted between C and go", func(t *testing.T) {
holder := toPositionResponse(gpr)
test.That(t, holder.ComponentReference, test.ShouldEqual, "C++ component reference")

test.That(t, holder.X, test.ShouldEqual, 100)
test.That(t, holder.Y, test.ShouldEqual, 200)
Expand Down Expand Up @@ -342,7 +341,6 @@ func TestCGoAPIWithoutMovementSensor(t *testing.T) {
// test position zeroed
position, err = vc.position()
test.That(t, err, test.ShouldBeNil)
test.That(t, position.ComponentReference, test.ShouldEqual, "my-lidar")
positionIsZero(t, position)

// test pointCloudMap now returns a non empty result
Expand All @@ -369,7 +367,6 @@ func TestCGoAPIWithoutMovementSensor(t *testing.T) {
// test position, is no longer zeroed
position, err = vc.position()
test.That(t, err, test.ShouldBeNil)
test.That(t, position.ComponentReference, test.ShouldEqual, "my-lidar")
test.That(t, position.X, test.ShouldNotEqual, 0)
test.That(t, position.Y, test.ShouldNotEqual, 0)
test.That(t, position.Z, test.ShouldEqual, 0)
Expand Down Expand Up @@ -604,7 +601,6 @@ func TestCGoAPIWithMovementSensor(t *testing.T) {
// test position is not zeroed
position, err = vc.position()
test.That(t, err, test.ShouldBeNil)
test.That(t, position.ComponentReference, test.ShouldEqual, "my-lidar")
test.That(t, r3.Vector{X: position.X, Y: position.Y, Z: position.Z}, test.ShouldNotResemble, r3.Vector{X: 0, Y: 0, Z: 0})

// test pointCloudMap returns a non empty result
Expand Down
15 changes: 1 addition & 14 deletions viam-cartographer/src/carto_facade/carto_facade.cc
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,6 @@ config from_viam_carto_config(viam_carto_config vcc) {
throw VIAM_CARTO_COMPONENT_REFERENCE_INVALID;
}
validate_lidar_config(c.lidar_config);
c.component_reference = bstrcpy(vcc.camera);

return c;
};
Expand Down Expand Up @@ -179,7 +178,7 @@ CartoFacade::CartoFacade(viam_carto_lib *pVCL, const viam_carto_config c,
path_to_internal_state_file = config.existing_map;
};

CartoFacade::~CartoFacade() { bdestroy(config.component_reference); }
CartoFacade::~CartoFacade() { }

void CartoFacade::IOInit() {
if (state != CartoFacadeState::INITIALIZED) {
Expand Down Expand Up @@ -538,7 +537,6 @@ void CartoFacade::GetPosition(viam_carto_get_position_response *r) {
r->imag = pos_quat.x();
r->jmag = pos_quat.y();
r->kmag = pos_quat.z();
r->component_reference = bstrcpy(config.component_reference);
};

void CartoFacade::GetPointCloudMap(viam_carto_get_point_cloud_map_response *r) {
Expand Down Expand Up @@ -642,11 +640,6 @@ void CartoFacade::AddLidarReading(const viam_carto_lidar_reading *sr) {
<< CartoFacadeState::STARTED;
throw VIAM_CARTO_NOT_IN_STARTED_STATE;
}
if (biseq(config.component_reference, sr->lidar) == false) {
VLOG(1) << "expected sensor: " << to_std_string(sr->lidar) << " to be "
<< config.component_reference;
throw VIAM_CARTO_UNKNOWN_SENSOR_NAME;
}
std::string lidar_reading = to_std_string(sr->lidar_reading);
if (lidar_reading.length() == 0) {
throw VIAM_CARTO_LIDAR_READING_EMPTY;
Expand Down Expand Up @@ -1135,12 +1128,6 @@ extern int viam_carto_get_position_response_destroy(
return VIAM_CARTO_GET_POSITION_RESPONSE_INVALID;
}
int return_code = VIAM_CARTO_SUCCESS;
int rc = BSTR_OK;
rc = bdestroy(r->component_reference);
if (rc != BSTR_OK) {
return_code = VIAM_CARTO_DESTRUCTOR_ERROR;
}
r->component_reference = nullptr;
return return_code;
};

Expand Down
5 changes: 1 addition & 4 deletions viam-cartographer/src/carto_facade/carto_facade.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,6 @@ typedef struct viam_carto_get_position_response {
double imag;
double jmag;
double kmag;

bstring component_reference;
} viam_carto_get_position_response;

typedef struct viam_carto_get_point_cloud_map_response {
Expand Down Expand Up @@ -372,7 +370,6 @@ static const double resolutionMeters = 0.05;
typedef struct config {
std::string camera;
std::string movement_sensor;
bstring component_reference;
viam_carto_LIDAR_CONFIG lidar_config;
bool enable_mapping;
std::string existing_map;
Expand Down Expand Up @@ -411,7 +408,7 @@ class CartoFacade {

// GetPosition returns the relative pose of the robot w.r.t the "origin"
// of the map, which is the starting point from where the map was initially
// created along with a component reference.
// created
void GetPosition(viam_carto_get_position_response *r);

// GetPointCloudMap returns a stream of the current sampled pointcloud
Expand Down
12 changes: 0 additions & 12 deletions viam-cartographer/src/carto_facade/carto_facade_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,6 @@ BOOST_AUTO_TEST_CASE(CartoFacade_init_validate) {

// TODO: Move all suite level setup & teardown to boost test hook
// Teardown
viam_carto_config_teardown(vcc_empty_component_ref);
viam_carto_config_teardown(vcc_invalid_lidar_config);
viam_carto_config_teardown(vcc_invalid_movement_sensor_config);
viam_carto_config_teardown(vcc_with_movement_sensor_succ);
Expand Down Expand Up @@ -490,7 +489,6 @@ BOOST_AUTO_TEST_CASE(CartoFacade_init_terminate_without_movement_sensor) {
BOOST_TEST(((cf->state) == CartoFacadeState::IO_INITIALIZED));
BOOST_TEST((cf->config.camera) == camera);
BOOST_TEST((cf->config.movement_sensor) == movement_sensor);
BOOST_TEST(to_std_string(cf->config.component_reference) == "lidar");
BOOST_TEST((cf->config.lidar_config) == VIAM_CARTO_THREE_D);

BOOST_TEST(viam_carto_terminate(&vc) == VIAM_CARTO_SUCCESS);
Expand Down Expand Up @@ -764,7 +762,6 @@ BOOST_AUTO_TEST_CASE(CartoFacade_demo_without_movement_sensor) {
BOOST_TEST(pr.jmag == 0);
BOOST_TEST(pr.kmag == 0);
BOOST_TEST(pr.real == 1);
BOOST_TEST(to_std_string(pr.component_reference) == "lidar");

BOOST_TEST(viam_carto_get_position_response_destroy(&pr) ==
VIAM_CARTO_SUCCESS);
Expand Down Expand Up @@ -820,7 +817,6 @@ BOOST_AUTO_TEST_CASE(CartoFacade_demo_without_movement_sensor) {
BOOST_TEST(pr.jmag == 0);
BOOST_TEST(pr.kmag != 0);
BOOST_TEST(pr.real != 1);
BOOST_TEST(to_std_string(pr.component_reference) == "lidar");

BOOST_TEST(viam_carto_get_position_response_destroy(&pr) ==
VIAM_CARTO_SUCCESS);
Expand Down Expand Up @@ -907,14 +903,12 @@ BOOST_AUTO_TEST_CASE(CartoFacade_config_without_movement_sensor) {

struct config c = viam::carto_facade::from_viam_carto_config(vcc);

BOOST_TEST(to_std_string(c.component_reference) == "lidar");
BOOST_TEST(c.lidar_config == VIAM_CARTO_THREE_D);
BOOST_TEST(c.camera == "lidar");
BOOST_TEST(c.movement_sensor == "");
BOOST_TEST(c.enable_mapping == true);

viam_carto_config_teardown(vcc);
BOOST_TEST(bdestroy(c.component_reference) == BSTR_OK);

// library terminate
BOOST_TEST(viam_carto_lib_terminate(&lib) == VIAM_CARTO_SUCCESS);
Expand Down Expand Up @@ -993,7 +987,6 @@ BOOST_AUTO_TEST_CASE(CartoFacade_init_terminate_with_movement_sensor) {
BOOST_TEST(((cf->state) == CartoFacadeState::IO_INITIALIZED));
BOOST_TEST((cf->config.camera) == camera);
BOOST_TEST((cf->config.movement_sensor) == movement_sensor);
BOOST_TEST(to_std_string(cf->config.component_reference) == "lidar");
BOOST_TEST((cf->config.lidar_config) == VIAM_CARTO_THREE_D);

BOOST_TEST(viam_carto_terminate(&vc) == VIAM_CARTO_SUCCESS);
Expand Down Expand Up @@ -1346,7 +1339,6 @@ BOOST_AUTO_TEST_CASE(CartoFacade_demo_with_movement_sensor) {
BOOST_TEST(pr.jmag != 0);
BOOST_TEST(pr.kmag != 0);
BOOST_TEST(pr.real != 1);
BOOST_TEST(to_std_string(pr.component_reference) == "lidar");

BOOST_TEST(viam_carto_get_position_response_destroy(&pr) ==
VIAM_CARTO_SUCCESS);
Expand Down Expand Up @@ -1402,7 +1394,6 @@ BOOST_AUTO_TEST_CASE(CartoFacade_demo_with_movement_sensor) {
BOOST_TEST(pr.jmag != 0);
BOOST_TEST(pr.kmag != 0);
BOOST_TEST(pr.real != 1);
BOOST_TEST(to_std_string(pr.component_reference) == "lidar");

BOOST_TEST(viam_carto_get_position_response_destroy(&pr) ==
VIAM_CARTO_SUCCESS);
Expand Down Expand Up @@ -1459,7 +1450,6 @@ BOOST_AUTO_TEST_CASE(CartoFacade_demo_with_movement_sensor) {
BOOST_TEST(pr.jmag != 0);
BOOST_TEST(pr.kmag != 0);
BOOST_TEST(pr.real != 1);
BOOST_TEST(to_std_string(pr.component_reference) == "lidar");

BOOST_TEST(viam_carto_get_position_response_destroy(&pr) ==
VIAM_CARTO_SUCCESS);
Expand Down Expand Up @@ -1589,14 +1579,12 @@ BOOST_AUTO_TEST_CASE(CartoFacade_config_with_movement_sensor) {

struct config c = viam::carto_facade::from_viam_carto_config(vcc);

BOOST_TEST(to_std_string(c.component_reference) == "lidar");
BOOST_TEST(c.lidar_config == VIAM_CARTO_THREE_D);
BOOST_TEST(c.camera == "lidar");
BOOST_TEST(c.movement_sensor == "movement_sensor");
BOOST_TEST(c.enable_mapping == true);

viam_carto_config_teardown(vcc);
BOOST_TEST(bdestroy(c.component_reference) == BSTR_OK);

// library terminate
BOOST_TEST(viam_carto_lib_terminate(&lib) == VIAM_CARTO_SUCCESS);
Expand Down
13 changes: 6 additions & 7 deletions viam_cartographer.go
Original file line number Diff line number Diff line change
Expand Up @@ -489,12 +489,11 @@ func initCartoFacade(ctx context.Context, cartoSvc *CartographerService) error {
}

cartoCfg := cartofacade.CartoConfig{
Camera: cartoSvc.lidar.Name(),
MovementSensor: movementSensorName,
ComponentReference: cartoSvc.lidar.Name(),
LidarConfig: cartofacade.TwoD,
EnableMapping: cartoSvc.enableMapping,
ExistingMap: cartoSvc.existingMap,
Camera: cartoSvc.lidar.Name(),
MovementSensor: movementSensorName,
LidarConfig: cartofacade.TwoD,
EnableMapping: cartoSvc.enableMapping,
ExistingMap: cartoSvc.existingMap,
}

cf := cartofacade.New(&cartoLib, cartoCfg, cartoAlgoConfig)
Expand Down Expand Up @@ -576,7 +575,7 @@ type CartographerService struct {
}

// Position forwards the request for positional data to the slam library's gRPC service. Once a response is received,
// it is unpacked into a Pose and a component reference string.
// it is unpacked into a Pose.
func (cartoSvc *CartographerService) Position(ctx context.Context) (spatialmath.Pose, error) {
ctx, span := trace.StartSpan(ctx, "viamcartographer::CartographerService::Position")
defer span.End()
Expand Down

0 comments on commit bc0e97d

Please sign in to comment.