Add support for MPU6050 accelerometer/gyroscope [RSDK-868] #1611
Conversation
|
This PR is uncomfortably long. For next time, do you see a way I should have split it up into smaller pieces? |
viambot
added
safe to test
|
(The changes to the .js files are solely from running |
There was a problem hiding this comment.
This PR size is fine for adding a model. There are no worries there, it's just the amount of code that goes into writing a driver. Do not worry about the frontend folder either.
It's looking good, done with first pass. A few nits here and there, and one major question about when we poll the sensor for information/how continuously we poll. Right now we're limited by snapshots from when we request AngularVelocity and Readings, and not polling continuously in the background.
viambot
added
safe to test
|
|
||
| func (mpu *mpu6050) Readings(ctx context.Context, extra map[string]interface{}) (map[string]interface{}, error) { | ||
| mpu.mu.Lock() | ||
| defer mpu.mu.Lock() |
There was a problem hiding this comment.
🤦 Well there's yer problem! After calling this function, the mutex never gets unlocked, which means no other values can be read afterwards.
There was a problem hiding this comment.
...and it's worse than that! By deferring a second Lock(), we have a deadlock in the deferred part of the function, which is why we don't even get a response from the first call to the readings.
viambot
added
safe to test
|
Sorry about the delay getting back to this. I've added in the background goroutine you suggested, and we can now get all the readings from the chip without it locking up. Take another look! |
viambot
added
safe to test
viambot
added
safe to test
|
Background goroutine refactored! My only lingering doubt is that the temperature sensor always reads 35-37 degrees Celcius, whereas I'd expect something closer to 23. This chip really shouldn't be running hot, should it? and yet, page 30 of this PDF sure implies that I've got the values correct. Maybe the thermometer isn't nearly as reliable as I had expected? I'm inclined to merge this even if the thermometer does weird things. If there's a bug, we can deal with it later. |
|
...okay, one more doubt: github's CI complains that " |
viambot
added
safe to test
|
Agreed. The thermometer on a gyro isn't a priority. And if the manufacturer themselves have a published error, we shouldn't guess at the right scaling. |
viambot
added
safe to test
|
Tried with the chip on a breadboard wired to a raspberry pi: seems to work adequately. We only get updates twice per second on app.viam.com (and only angular velocity), but in theory you should be able to go much faster, and thereby get more interesting data from writing your own app.
There's something wrong with getting the raw readings: I had expected to be able to see the temperature and linear acceleration via the app, but that's not quite working yet. but I figured I should get the PR out to get more eyes looking at it, even before I've finished tweaking the final details.