Skip to content

Fix qmc5883l lockup #13467

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Mar 21, 2024
Merged

Fix qmc5883l lockup #13467

merged 1 commit into from
Mar 21, 2024

Conversation

ledvinap
Copy link
Contributor

Data registers are locked until last/unlock register is read. New data are not stored when locked and DRDY is not set.
On bus error, read may finish early (not reading unlock register) and thus cause driver lockup.

Fix qmc5883l lockup
5126059 
Data registers are locked until last/unlock register is read. New data
are not stored when locked and DRDY is not set.
On bus error, read may finish early (not reading unlock register) and thus
cause driver lockup.
@github-actions GitHub Actions
Copy link

Do you want to test this code? You can flash it directly from Betaflight Configurator:

  • Simply put #13467 (this pull request number) in the Select commit field of the Configurator firmware flasher tab (you need to Enable expert mode, Show release candidates and Development).

WARNING: It may be unstable. Use only for testing!

@ledvinap ledvinap mentioned this pull request Mar 21, 2024
Open
@haslinghuis
Copy link
Member

haslinghuis commented Mar 21, 2024 •
edited
Loading

Related or not?

  • Build on IFLIGHT_BLITZ_F435 with USE_MAG included the BARO get's stuck here:

betaflight/src/main/sensors/barometer.c

Lines 461 to 466 in b9b4b77

case BARO_STATE_PRESSURE_SAMPLE:
if (!baro.dev.get_up(&baro.dev)) {
// No action was taken as the read has not completed
schedulerIgnoreTaskExecTime();
break;
}

Tested on master and this PR using cloud build.

@haslinghuis haslinghuis added this to the 4.5 milestone Mar 21, 2024
@ledvinap
Copy link
Contributor Author

  • Build on IFLIGHT_BLITZ_F435 with USE_MAG included the BARO get's stuck here:

    betaflight/src/main/sensors/barometer.c

    Lines 461 to 466 in b9b4b77

    case BARO_STATE_PRESSURE_SAMPLE:
    if (!baro.dev.get_up(&baro.dev)) {
    // No action was taken as the read has not completed
    schedulerIgnoreTaskExecTime();
    break;
    }

Tested on master and this PR using cloud build. 🤯 using make IFLIGHT_BLITZ_F435 and local flash it works 😦

What baro is configured/detected? Is qmc5883l used on that target?

@haslinghuis
Copy link
Member

Baro is DPS310, but there is no physical mag attached to the board. So unrelated to this - but peculiar

@ledvinap
Copy link
Contributor Author

Baro is DPS310, but there is no physical mag attached to the board. So unrelated to this - but peculiar

That is strange - https://github.com/betaflight/betaflight/blob/master/src/main/drivers/barometer/barometer_dps310.c#L243 should always return true, it should end up in BARO_STATE_PRESSURE_READ

And don't expect anything from HAL drivers when hardware error occurs.

haslinghuis
haslinghuis reviewed Mar 21, 2024
View reviewed changes
src/main/drivers/compass/compass_qmc5883l.c
#define QMC5883L_REG_STATUS 0x06
#define QMC5883L_REG_STATUS_DRDY 0x01
#define QMC5883L_REG_STATUS_OVL 0x02
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps also handle data overflow:

image

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Output values are saturated, so OVL bit caries little extra information.

@haslinghuis haslinghuis merged commit 2a6ae06 into betaflight:master Mar 21, 2024
@ledvinap ledvinap deleted the fix-qmc5883l branch March 22, 2024 10:11
@FHermisch
Copy link

Some days ago I also worked on the issue of a not responding qmc5883l (data stuck in sensors tab). I first also worked on the DOR issue, but this did not completely solve my problem (but maybe my implementation was just not correct in doing so). I fixed it for me now, but this included some changes on the hardware interrupt handling and forcing some re-initialization.
If you still face issues of stuck data, reach out and we can try to combine our fixes.

mluessi pushed a commit to BrainFPV/betaflight that referenced this pull request Mar 26, 2024
@ledvinap @mluessi
Fix qmc5883l lockup (betaflight#13467)
c1e41f1 
Data registers are locked until last/unlock register is read. New data
are not stored when locked and DRDY is not set.
On bus error, read may finish early (not reading unlock register) and thus
cause driver lockup.

Co-authored-by: Petr Ledvina <ledvinap@hp124.ekotip.cz>
@Giammi Giammi mentioned this pull request Apr 1, 2024
Merged
@ctzsnooze
Copy link
Member

Can I ask how this PR is progressing? I can't test it myself.

@lida2003 lida2003 mentioned this pull request Apr 6, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@blckmn blckmn blckmn approved these changes

@haslinghuis haslinghuis haslinghuis approved these changes

@nerdCopter nerdCopter nerdCopter approved these changes

@McGiverGim McGiverGim Awaiting requested review from McGiverGim

@SteveCEvans SteveCEvans Awaiting requested review from SteveCEvans

@ctzsnooze ctzsnooze Awaiting requested review from ctzsnooze

@KarateBrot KarateBrot Awaiting requested review from KarateBrot

@klutvott123 klutvott123 Awaiting requested review from klutvott123

@sugaarK sugaarK Awaiting requested review from sugaarK

Assignees

@ledvinap ledvinap

Labels
RN: BUGFIX
Projects
Finalizing Firmware 4.5 Release
Status: COMPLETED
Milestone
4.5
Development

Successfully merging this pull request may close these issues.
6 participants
@ledvinap @haslinghuis @FHermisch @ctzsnooze @blckmn @nerdCopter