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

corrected MPU6050 WHOAMI as per issue #112 #113

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

wallarug
Copy link

@wallarug wallarug commented Feb 7, 2020

As above.

@jamesadevine
Copy link
Contributor

jamesadevine commented Feb 7, 2020

Hiya @wallarug,

Thank you for the bug report and corresponding pull request, it is greatly appreciated.

A cursory look over the data sheet for the MPU6050 does indeed state that the slave address is 0x68, and that the who am i should return bits 1 through 6 of the slave address:
___-xxx xxx-
0b0110 1000

Have you confirmed that this works on a device? I do not have an MPU6050 to hand.

@wallarug
Copy link
Author

wallarug commented Feb 7, 2020

Hey @jamesadevine ,

I cannot test at this very moment but am very confident that it should be 0x68 and not 0x34.

I haven't tested with this specific library but have been writing Python Drivers for the chip and it's relatives for the past few months.

It clearly says, as you stated, it should return 0x68 from the datasheet:
image

I can also confirm the corresponding cpp file says it in a comment 0x68 for this same repo:
https://github.com/lancaster-university/codal-core/blob/master/source/drivers/MPU6050.cpp#L47

@jamesadevine
Copy link
Contributor

@wallarug I absolutely believe you to be correct, but sometimes merging a PR that has a seemingly minor change can have big ramifications, hence my hesitation. For instance, the MPU6050 driver is currently in use in MakeCode Arcade, a small change here may cause functionality regression in the future and impact quite a lot of users.

@mmoskal thoughts? I'm happy if you're happy.

@wallarug
Copy link
Author

wallarug commented Feb 7, 2020

That's how I discovered the issue in the first place. I am trying to use an IMU in MakeCode but the code doesn't run on the board at all - just crashes because the WHOAMI is wrong (be it - I am using an MPU9250 which has a different DEVICE_ID anyway).

Up to you if you want to accept or reject this change.

@jamesadevine
Copy link
Contributor

Lol! That's good to know. Thanks for taking the time to investigate fully 😄

@mmoskal
Copy link
Contributor

mmoskal commented Feb 7, 2020

Anyone knows why this code is here: https://github.com/lancaster-university/codal-core/blob/master/source/drivers/MPU6050.cpp#L52 ? @xmeow ?

Also MakeCode just tries to read the WHOAMI register, but doesn't look at the value, so I'm not sure why would it be crashing.

@jamesadevine
Copy link
Contributor

@mmoskal I think that operation would make sense if the who am i register returned the LSB of the address. I guess semantically it returns whether or not the device is of the MPU60X0 range.

Still confusing though...

@wallarug
Copy link
Author

wallarug commented Feb 8, 2020

So did anyone test the original code before it was put into the repository?

This seems like a very small impact change - particularly if @mmoskal is saying that it's not even used - which I find very odd and will probably investigate further.

The normal process using these IMU / chips is that you do the following:

1.  Reset chip
2.  Check ID of chip
3.  Setup chip
4.  Use chip

The Check ID step is important because there are different ones out there (including fakes) that may use slightly different registers. Its a bit dangerous to not check.

I'm going to add in the MPU6500 (for eventual MPU9250 support) later this month after I test it with MakeCode.

@wallarug
Copy link
Author

Hey @jamesadevine

Is there a way I can test this in MakeCode without pushing it into the repo first?

What would be the best setup for compiling and testing code to go into this repository using MakeCode? I cannot see a clear link between microsoft/pxt-maker and the compiled code that comes from this repository. There is an obvious dependency but I cannot work out how to setup a testing environment.

Any help would be appreciated.

@pelikhan
Copy link
Contributor

If you look at the pxtarget.json in pxt-maker you will see which version of Codal is used for each MCU. These version of Codal eventually bind to a version of Codal-core.

If you run

pxt buildtarget —local

it will then locally and you’ll get the full Codal layout to build things locally too.

@wallarug
Copy link
Author

wallarug commented Feb 14, 2020

@pelikhan thank you. Where do the files end up in the makecode structure?

pxt
pxt-maker
pxt-common-package

I want to know where the files from this repository (codal-core) end up on my computer so we can change them for testing.

@wallarug
Copy link
Author

wallarug commented Feb 15, 2020

@pelikhan @jamesadevine Thank you for assistance.

pxt serve --local does the trick.

I have now located the files and am working out how to test things with different boards. The updated docs also helped greatly. I will test out this PR and report back with results over the next few days.

It does a long time if you select to do all boards. My Intel i7-4790k is getting a work out.

@pelikhan
Copy link
Contributor

Yeah “pxt serve —local” is very slow. Best work on a single board first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants