Skip to content

Conversation

@cal-pratt
Copy link
Contributor

This PR adds the AltIMU to the Rov class. It also fixes #168 by reading the AltIMU (and voltage and current) sensors on their own Observable.

Details:
In order to test the Rov class, the AltIMU must be able to mocked. The mocked class must not rely on I2C and needs to have some interface which can be implemented by the test code. Following a similar strategy to the Maestro code, the Rov requests a generic IMU which implements 5 sensor interfaces.

This change removes the dependency between the AltIMU and rx observables by only exposing simple get methods (linking these to an rx observable can be done from the rov class itself).

Future work will be creating proper tests in RovTest and RovSimulator classes

@cal-pratt cal-pratt added this to the MATE 2017 milestone Jan 22, 2017
@cal-pratt cal-pratt self-assigned this Jan 22, 2017
@cal-pratt cal-pratt force-pushed the generic-imu branch 2 times, most recently from e8059ea to 2e8e621 Compare January 23, 2017 22:13
@cal-pratt cal-pratt mentioned this pull request Jan 25, 2017
@cal-pratt
Copy link
Contributor Author

Update

I reworked the IMU class such that it can be tested. The I2C bus passed to the IMU is now a list of I2C devices. This allows us to create a mock set of i2c devices that can be used to test the IMU reading/setting registers. the I2C class has also been made thread safe in the event that we want to read one sensor more than another. (ie, temperature could probably be slower than gyro data). RovTest and RovSimulator still need proper tests defined, however, the IMU itself now has adequate test coverage! Should be good to merge once it gets tested on the actual ROV

Copy link
Contributor

@ConnorWhalen ConnorWhalen left a comment

Choose a reason for hiding this comment

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

LooksGoodToMe, just test it on the ROV as you mentioned

@cal-pratt cal-pratt merged commit cfec713 into master Feb 4, 2017
@cal-pratt cal-pratt deleted the generic-imu branch February 4, 2017 18:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Sensor polling should be done separate to thruster updates

3 participants