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.

Sign up for GitHub

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

Jump to bottom

Probe for rm3100 #10581

Closed
wants to merge 3 commits into from
Closed

Probe for rm3100 #10581

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
401c8b5
AP_Compass: probe for RM3100
khancyr Feb 20, 2019
97db19e
AP_Compass: add corection for CMM configuration and correct debuging msg
khancyr Feb 20, 2019
1eae546
AP_Compass: RM3100, don't touch TMRC register to use defualt CMM upda…
khancyr Feb 22, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion libraries/AP_Compass/AP_Compass.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -617,7 +617,7 @@ void Compass::_probe_external_i2c_compasses(void)
all_external,
all_external?HAL_COMPASS_QMC5883L_ORIENTATION_EXTERNAL:HAL_COMPASS_QMC5883L_ORIENTATION_INTERNAL));
}

#if !HAL_MINIMIZE_FEATURES
// AK09916 on ICM20948
FOREACH_I2C_EXTERNAL(i) {
Expand Down Expand Up @@ -693,6 +693,17 @@ void Compass::_probe_external_i2c_compasses(void)
ADD_BACKEND(DRIVER_IST8308, AP_Compass_IST8308::probe(GET_I2C_DEVICE(i, HAL_COMPASS_IST8308_I2C_ADDR),
true, ROTATION_NONE));
}

// external i2c bus
FOREACH_I2C_EXTERNAL(i) {
ADD_BACKEND(DRIVER_RM3100, AP_Compass_RM3100::probe(GET_I2C_DEVICE(i, HAL_COMPASS_RM3100_I2C_ADDR),
true, ROTATION_NONE));
}
FOREACH_I2C_INTERNAL(i) {
Copy link
Contributor

Choose a reason for hiding this comment

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

why are we probing interbal bus? for internal it should be an explicit line in hwdef.dat

Copy link

Choose a reason for hiding this comment

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

Excuse my lack of understanding but how does this differ from all the other internal i2c probes in this file?

ADD_BACKEND(DRIVER_RM3100, AP_Compass_RM3100::probe(GET_I2C_DEVICE(i, HAL_COMPASS_RM3100_I2C_ADDR),
all_external, ROTATION_NONE));
}

#endif // HAL_MINIMIZE_FEATURES
}

Expand Down
13 changes: 5 additions & 8 deletions libraries/AP_Compass/AP_Compass_RM3100.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,13 +117,14 @@ bool AP_Compass_RM3100::init()
ccy1 != CCP1_DEFAULT || ccy0 != CCP0_DEFAULT ||
ccz1 != CCP1_DEFAULT || ccz0 != CCP0_DEFAULT) {
// couldn't read one of the cycle count registers or didn't recognize the default cycle count values
goto fail;
Copy link
Contributor

Choose a reason for hiding this comment

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

this change doesn't seem to do anything?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it just remove the goto that is useless

// hal.console->printf("RM3100: Unable to read the register for whoami test\n");
Copy link
Member

Choose a reason for hiding this comment

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

Left-over?

dev->get_semaphore()->give();
return false;
}

dev->setup_checked_registers(8);

dev->write_register(RM3100_TMRC_REG, TMRC, true); // cycle count z
dev->write_register(RM3100_CMM_REG, CMM, false); // CMM configuration
dev->write_register(RM3100_CMM_REG, CMM, true); // CMM configuration
Copy link
Contributor

Choose a reason for hiding this comment

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

have you flight tested the use of CMM? If so, can you please give a flight log?
It does look like we should enable it, but it needs testing, and we should use a valid TMRC value.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I use this code to test the sensors, and the reading looks fine and agree with other compass. Surely with wrong TMRC, it get back to default value . here are some logs with the RM3100 as compass2 with this code : https://drive.google.com/open?id=19pSJBCCyfYx-mhPedg6KyBzomxzbfQEy

dev->write_register(RM3100_CCX1_REG, CCP1, true); // cycle count x
dev->write_register(RM3100_CCX0_REG, CCP0, true); // cycle count x
dev->write_register(RM3100_CCY1_REG, CCP1, true); // cycle count y
Expand All @@ -141,7 +142,7 @@ bool AP_Compass_RM3100::init()
/* register the compass instance in the frontend */
compass_instance = register_compass();

printf("Found a RM3100 at address 0x%x as compass %u\n", dev->get_bus_address(), compass_instance);
hal.console->printf("RM3100: Found at address 0x%x as compass %u\n", dev->get_bus_address(), compass_instance);

set_rotation(compass_instance, rotation);

Expand All @@ -157,10 +158,6 @@ bool AP_Compass_RM3100::init()
FUNCTOR_BIND_MEMBER(&AP_Compass_RM3100::timer, void));

return true;

fail:
dev->get_semaphore()->give();
return false;
}

void AP_Compass_RM3100::timer()
Expand Down