universam1 / iSpindel

electronic Hydrometer
http://www.ispindel.de
Other
817 stars 326 forks source link

Added support for MPU6500 basic functionality #581

Closed Bananamannn closed 2 years ago

Bananamannn commented 2 years ago

Will need to be double checked using a MPU6050 as I don't have one on hand. However MPU6500 seems to work just fine

mp-se commented 2 years ago

You need to patch the MPU6050.cpp file as well or it will fail on this function; accelgyro.testConnection(). This code will work;

bool MPU6050_Base::testConnection() { return getDeviceID() == 0x34 || getDeviceID() == 0x38; // Allow both MPU6050 and MPU6000 }

Bananamannn commented 2 years ago

You need to patch the MPU6050.cpp file as well or it will fail on this function; accelgyro.testConnection(). This code will work;

bool MPU6050_Base::testConnection() { return getDeviceID() == 0x34 || getDeviceID() == 0x38; // Allow both MPU6050 and MPU6000 }

I didn't want to make any changes to the standard MPU6050 repository so I tried to remove the "accelgyro.testConnection()" line in the main ISpindel.cpp file and replace it so that function was never used. Did I miss it somewhere else other than the repo?

mp-se commented 2 years ago

You need to patch the MPU6050.cpp file as well or it will fail on this function; accelgyro.testConnection(). This code will work; bool MPU6050_Base::testConnection() { return getDeviceID() == 0x34 || getDeviceID() == 0x38; // Allow both MPU6050 and MPU6000 }

I didn't want to make any changes to the standard MPU6050 repository so I tried to remove the "accelgyro.testConnection()" line in the main ISpindel.cpp file and replace it so that function was never used. Did I miss it somewhere else other than the repo?

Sorry, my misstake. didnt look at the changes. just noticed that there was a call to that method in the code.

thegreatgunbantoad commented 2 years ago

Is it worth altering that if statement a bit by adding brackets so as to not rely on operation order i.e.: if ( (id == 0x34) || (id == 0x38) )

universam1 commented 2 years ago

Is it worth altering that if statement a bit by adding brackets so as to not rely on operation order i.e.: if ( (id == 0x34) || (id == 0x38) )

https://docs.microsoft.com/en-us/cpp/c-language/precedence-and-order-of-evaluation?view=msvc-170 parentheses are not necessary here

thegreatgunbantoad commented 2 years ago

I know they aren't strictly needed, it's more of a good practise / style kind of thing, also for readability.

Bananamannn commented 2 years ago

Linter : code formatter fails

I honestly have no idea what this is. I haven't changed this from your master branch. Any ideas why it's failing on here but not when I build it on my PC?

Bananamannn commented 2 years ago

Could I please get some advice on how to deal with the Linter fail, any pointer would be greatly appreciated. I honestly have no idea why it keeps failing.

https://github.com/universam1/iSpindel/runs/6349107436?check_suite_focus=true

thegreatgunbantoad commented 2 years ago

Is the dark green stuff in the files changed bit the stuff the linter doesn't like? I can't find the results of the linter. So possibly a space after the 2nd comma in main.yml line 3 and one less space before the if in ispindel.cpp line 974

Bananamannn commented 2 years ago

It's in the link below. I added the linter to my fork for testing. The red and green are the changes that i've made.

https://github.com/Bananamannn/iSpindel/runs/6418152846?check_suite_focus=true

image

pppedrillo commented 2 years ago

@Bananamannn All you had to to to get rid of format errors is just execute "Format document" command in VS Code (assuming you have clang-format installed by default)

Capture

Bananamannn commented 2 years ago

@Bananamannn All you had to to to get rid of format errors is just execute "Format document" command in VS Code (assuming you have clang-format installed by default)

Oh right, thanks for the info. I'll defiantly do it that way next time. I did it all through the Github editor this round.