arduino / ArduinoCore-arc32

GNU Lesser General Public License v2.1
329 stars 284 forks source link

Orientation Visualizer doesn't show pitch & roll properly #151

Open PaulStoffregen opened 8 years ago

PaulStoffregen commented 8 years ago

The Processing-based orientation visualizer examples doesn't display properly. Pitch and roll are only shown independently when yaw is 0 or 180 degrees. When yaw is 90 degrees, they display the same!

I made this video demo to show the problem:

https://www.youtube.com/watch?v=SHJ2n1Kwe58

Here is the page where the orientation visualizer code appears.

https://www.arduino.cc/en/Tutorial/Genuino101CurieIMUOrientationVisualiser

PaulStoffregen commented 8 years ago

I've created a fixed version.

https://github.com/arduino-libraries/MadgwickAHRS/blob/master/extras/Visualizer/Visualizer.pde

eriknyquist commented 8 years ago

Thanks @PaulStoffregen, I'll look at integrating your corrected version.

eriknyquist commented 8 years ago

@PaulStoffregen sorry about the delay on this, I finally got round to trying this out today. You're right, the Processing sketch given produces wrong movement for me too. However, when I tried your Processing sketch, I got no movement at all.

Since we've had a release since your original posting, would you mind trying this again with the latest corelibs, IDE and processing?

PaulStoffregen commented 8 years ago

Sorry, I put all that motion sensor stuff away a couple months ago, to focus on another really important project. I will look at this again, but probably not until after summer.

eriknyquist commented 8 years ago

OK, no worries I'll take a look at it.

bontempos commented 8 years ago

Hello, thanks guys for taking a look at it.. just to make sure I am not alone: Madgwick library available from arduino's official link is different from this, right?

Also, this new (4 month ago) library version will not compile, right?

eriknyquist commented 8 years ago

@bontempos, I am not sure about Paul's version of the library; I only tried "official" one from the Arduino tutorial. But, I'll try Paul's one and let you know what I get.

eriknyquist commented 8 years ago

This example is totally useless out-of-box, I think it's gotten even worse since @PaulStoffregen reported the original issue. Seems like the sketch was written to depend on the default ranges on the Gyro and Accel (setGyroRange() and setAccelerometerRange() are not used), and these defaults must have changed at some point since just moving the board a hair causes the model to spin violently on the screen.

So, I played around with different ranges, and these ones significantly improved the result;

CurieIMU.setGyroRange(2000); CurieIMU.setAccelerometerRange(16);

But, it seems that the issue Paul originally reported has since been obscured by additional issues. This whole tutorial probably needs a revamp.

PaulStoffregen commented 8 years ago

When I worked on this IMU stuff, back in March (2016), I created alternate Arduino & Processing sketches for the visualization. You can find them here:

https://github.com/arduino-libraries/MadgwickAHRS/blob/master/examples/Visualize101/Visualize101.ino

https://github.com/arduino-libraries/MadgwickAHRS/blob/master/extras/Visualizer/Visualizer.pde

Maybe these could be useful as a starting point for updating the tutorial & documentation?

I believe these are correct. Of course, they're based on the CurieIMU API which existed in March. Any problems or fixes should be submitted as issues or pull requests to the MadgwickAHRS repository.

Again, I do not plan to work on IMU stuff until at least October at the very earliest. My only purpose in this message is to point you to that work I did months ago, which might save you some time and avoid duplication of effort. Hope it helps?

PaulStoffregen commented 8 years ago

One more quick thing... these filter algorithms depend on the gyroscope data rate as well as its range. Even if the range is correct, if you alter the rate of data, you'll skew the results.

I know there is a tremendous desire to keep examples simple, both for the sake of the end user and also on the part of the authors writing them. But distributing examples to the Arduino world where the data rate collected from the gyroscope depends on the speed of the surrounding code and also the specific performance of the Wire library really does a disservice to novice end users. When they attempt to adapt the rate-sensitive example into their own projects, they'll almost certainly change the speed that loop() manages to run. Even if they preserve the rate, when/if the Wire library improves (or worsens) the rate may also change, causing the results to vary.

If you look at the example sketch I wrote for MagwickAHRS, there's an attempt to poll the millis() function and collect the IMU data at a fixed rate.

For the sake of users who will ultimately try to adapt the samples and tutorials, I hope you'll do something similar, so both the data range and data rate are well controlled.

eriknyquist commented 8 years ago

Yes, you're right. Really I want to make use of the BMI160s internal FIFOs to provide an IMU.available() function, as discussed in issue #150, I just haven't had time to do it yet. Appreciate the extra material, I'll check it out.

russmcinnis commented 8 years ago

The sketches Paul linked are not showing the issues from before so I think the demo is nice. I will run it past some others since I don't have much time with IMU. The image is fairly stable and I don't see the issues with the heading settings causing the pitch and roll to both control the pitch or something like that. I think we should use this.

kitsunami commented 7 years ago

@russmcinnis Can you verify if this is fixed in Castor or prior?

russmcinnis commented 7 years ago

I was able to see this all working back in August without any issues when it was assigned to me. If we got the correct processor sketch it was working. Because of some other issues I think this issue was left open. The latest activity was January 30, 2017

We had an issue once with a string/number conversion(dtostrf()) later which would cause jitter (Sept. 7, 2016). That was quickly fixed. Then Erik updated the sketch for some new IMU features added on January 23, 2017. "The CurieIMU library now has the features we were waiting for. I can update this sketch now and work the Arduino docs people this week to get this tutorial updated."

January 24, 2017 "And I have created a pull request in the Madgwick library to update the Visualize101.ino sketch it contains."

https://github.com/arduino-libraries/MadgwickAHRS/pull/16 January 30, 2017 "No updates-- still waiting for release to Arduino before changing public tutorial."

eriknyquist commented 7 years ago

@kitsunami please understand that the work to fix this issue has been done, we have the right sketch code for this tutorial to be updated. However, we can't change the public tutorial on arduino.cc until the next corelibs release is available in the IDE, since the sketch code wouldn't compile otherwise.

Since this ticket is specifically regarding that public tutorial not working, I am not going to close this until we're able to update the tutorial.

kitsunami commented 7 years ago

To be verified after Deneb release.

mjs513 commented 7 years ago

@eriknyquist and @PaulStoffregen (FYI). See this post on FIFO implementation. https://forum.arduino.cc/index.php?topic=462729.0