Pitch and roll values are reversed In real machine.

[mbitfun]

Describe the bug

In simulator the pitch and roll values are correct.
But, In real machine pitch and roll values are reversed

To Reproduce

Please check in real machine.

Desktop (please complete the following information):

• OS: Windows10
• Browser Chrome
• Version 68

[guillaumejenkins]

Good catch, this is a regression as it only happens in /beta

[guillaumejenkins]

@jaustin has something changed in the micro:bit framework around rotation? Our rotation API hasn't changed at all since v0. The functions are exactly the same:

v0: https://github.com/Microsoft/pxt-microbit/blob/master/libs/core/input.cpp#L310
v1: https://github.com/Microsoft/pxt-microbit/blob/v1/libs/core/input.cpp#L321

enum class Rotation {
    //% block=pitch
    Pitch = 0,
    //% block=roll
    Roll = 1,
};

int rotation(Rotation kind) {
    switch (kind) {
        case Rotation::Pitch: return uBit.accelerometer.getPitch();
        case Rotation::Roll: return uBit.accelerometer.getRoll();
    }
    return 0;
}

[guillaumejenkins]

Maybe @finneyj or @jamesadevine would have some info as well?

[jaustin]

Looks strongly like we broke this in the DAL, as the 'dal-integration' instance of v0 gets this wrong too.

@finneyj we should fix our axes!

DAL bug lancaster-university/microbit-dal#371

(PS this reminds me of #799 - which would have helped a lot here!)

[finneyj]

Thanks @guillaumejenkins, @jaustin. @mbitfun.

Curious - we definitely checked the axes were consistent after the patch. Will check again...

[finneyj]

Just built up two fresh C++ native test cases for the current HEAD revision of microbit-dal master, and the HEAD revision of dal-integration (which should translate to the versions compared above).

The accelerometer data coming out of the device driver via getX(), getY() and getZ() is consistent in terms of magnitude and axis alignment, so the issue must be constrained to the pitch and roll computation a little further up the stack... will dig some more.

[abchatra]

@finneyj any update here?

[finneyj]

hi @abchatra

yep - should be fixed in the dal-integration-2 beta...

[abchatra]

Thanks @finneyj We will validate and close this bug.

[finneyj]

thanks @abchatra. Let me know if you have any issues.

[guillaumejenkins]

@finneyj Jonny mentioned new config parameters for the build, to enable legacy pitch/roll. Where can I find info on those flags, and where must I enable them?

[finneyj]

@guillaumejenkins

Indeed, there's a compile time option that we should keep for v0. For v1 we'd like to move to the new full range values that make pitch and roll consistent with each other.

For legacy compat, you should set this #define to '0' :
https://github.com/lancaster-university/microbit-dal/blob/dal-integration/inc/core/MicroBitConfig.h#L329

there should be an option to allow you to flip this in your config.json file, but I'm afraid I forgot to add some glue in dal-integration-2 to let this happen.

Jonny and I are happy for the v0 beta to be a little inconsistent until the next release where we'll fix it, if that's ok with you?

[pelikhan]

@finneyj you mean we take this drop and introduce the small bug for v0. looks ok to me.

[finneyj]

@pelikhan

Yep, it's a very minor bug we can live with until the next drop.

[jaustin]

A couple if things: @pelikhan dal-integration-2 shouldn't be going to live yet, just v0-beta and v1-beta. I'd argue this is more incomplete fix than it is new bug, but it's still not the final state of affairs. @finneyj perhaps it's worth picking the yotta config name now so the MakeCode team can set the config in advance of the DAL arriving with it?

[finneyj]

I'd argue it's a bugfix. ;)

let's go with this for config.json

{
    "microbit-dal":
    {
        "full_range_pitch_calculation": 0
    }
}

[finneyj]

@pelikhan @guillaumejenkins

FYI, I've added the glue logic for this into the HEAD revision of the dal-integration branch, ready for the next tag.

lancaster-university/microbit-dal@83e1795

[guillaumejenkins]

@finneyj just to clarify, you only added the flag to dal-integration, not the full fix, correct? To test the fix we should still have our beta sites point to dal-integration-2?

[finneyj]

@guillaumejenkins HEAD revision of the dal-integration branch now contains the full fix, including the config.json glue that was missing yesterday. No tag on that yet though, pending other changes. If it's a big problem for you, let us know and i'll do an interim tag, but I'm happy to wait until we do another scheduled tag.

[pelikhan]

Could we get a quick tag for this fix? Get Outlook for iOS<https://aka.ms/o0ukef>

…

________________________________ From: Joe Finney <notifications@github.com> Sent: Thursday, September 6, 2018 8:27 AM To: Microsoft/pxt-microbit Cc: Peli de Halleux; Mention Subject: Re: [Microsoft/pxt-microbit] Pitch and roll values are reversed In real machine. (#1075) @guillaumejenkins<https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fguillaumejenkins&data=02%7C01%7Cjhalleux%40microsoft.com%7C87dde7c7bbe248bc936a08d6140d3fd8%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636718444471811045&sdata=jUFV82FHBOw2%2Bj1QxL%2F%2BeRy6CEytwOahjaQuyGt%2FZm0%3D&reserved=0> HEAD revision of the dal-integration branch now contains the full fix, including the config.json glue that was missing yesterday. No tag on that yet though, pending other changes. If it's a big problem for you, let us know and i'll do an interim tag, but I'm happy to wait until we do another scheduled tag. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FMicrosoft%2Fpxt-microbit%2Fissues%2F1075%23issuecomment-419136835&data=02%7C01%7Cjhalleux%40microsoft.com%7C87dde7c7bbe248bc936a08d6140d3fd8%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636718444471821054&sdata=975G2una4ydn1W2Xkr4YOdRmrnl7wZy65WBXu%2FJqlr0%3D&reserved=0>, or mute the thread<https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAD-4Kcgf_bN4ij0BS2b8e3wAOSblT6wOks5uYT7bgaJpZM4V5r2h&data=02%7C01%7Cjhalleux%40microsoft.com%7C87dde7c7bbe248bc936a08d6140d3fd8%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636718444471831062&sdata=Rq0GSNqjz1EGYmh43D2fMQqfZVlOQW5Oity%2F%2FpFAv5I%3D&reserved=0>.

[finneyj]

Just on the train with a draining battery… can it wait an hour?

[pelikhan]

Of course no worries.

[finneyj]

actually, I just made it with a few minute to spare. ;)

dal-integration-3

no time to test though, so please shout if you have problems

[guillaumejenkins]

Works well, thanks! We'll release this to our beta websites shortly

[finneyj]

great stuff. thanks @guillaumejenkins.

[guillaumejenkins]

Fix will be in the following releases:

• v0: v0.14.48
• v1: v1.1.47

[lock]

This thread has been automatically locked since there has not been
any recent activity after it was closed. Please open a new issue for
related bugs.