Skip to content

HAL: Add a get_capabilities() function to ResetReason API #12139

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 4 commits into from
Jan 16, 2020
Merged

HAL: Add a get_capabilities() function to ResetReason API #12139

merged 4 commits into from
Jan 16, 2020

Conversation

fkjagodzinski
Copy link
Member

@fkjagodzinski fkjagodzinski commented Dec 19, 2019 •
edited
Loading

Summary of changes

Add the hal_reset_reason_get_capabilities() function to the ResetReason HAL API to skip the unsupported reason values during HAL & driver tests.

Fixes #11792.

Updated tests:

  • tests-mbed_hal-reset_reason,
  • tests-mbed_drivers-reset_reason.

Impact of changes

Extend the ResetReason HAL API with the hal_reset_reason_get_capabilities() function. Unsupported reason values are skipped during the greentea tests.

Migration actions required

A default, weak implementation is provided. Every target has to override this weak implementation to provide the correct reset_reason_capabilities_t.

Documentation

None

Pull request type 

[] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[x] Feature update (New feature / Functionality change / New API)
[] Major update (Breaking change E.g. Return code change / API behaviour change)

Test results 

[] No Tests required for this change (E.g docs only update)
[x] Covered by existing mbed-os tests (Greentea or Unittest)
[] Tests / results supplied as part of this PR

Reviewers

@jamesbeyond, @mprse

@ciarmcom ciarmcom requested review from a team, jamesbeyond, maclobdell and mprse December 19, 2019 10:00
@ciarmcom
Copy link
Member

@fkjagodzinski, thank you for your changes.
@mprse @maclobdell @jamesbeyond @ARMmbed/mbed-os-core @ARMmbed/mbed-os-hal @ARMmbed/mbed-os-test @ARMmbed/mbed-os-maintainers please review.

@fkjagodzinski fkjagodzinski mentioned this pull request Dec 19, 2019
Closed
@adbridge
Copy link
Contributor

@fkjagodzinski What is the actual impact of these changes? Could you fill in that section also please.

@dustin-crossman
Copy link
Contributor

@fkjagodzinski Hey sorry I can't take a look at this before the holidays but I'll be sure to test out your changes as soon as I'm back.

@fkjagodzinski
Copy link
Member Author

@adbridge I've updated the description as requested.

@dustin-crossman
Copy link
Contributor

@fkjagodzinski I got around to testing this out. The changes look good to me. I was able to implement the get_capabilities function for our boards and get the tests passing.

0xc0170
0xc0170 reviewed Jan 3, 2020
View reviewed changes
Copy link
Contributor

@0xc0170 0xc0170 left a comment

Choose a reason for hiding this comment

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

Before I approve, what other targets/families should implement this (at least to check if weakly defined is good for them) ?

@0xc0170 0xc0170 added the release-version: 6.0.0-alpha-1 First pre-release version of 6.0.0 label Jan 3, 2020
Filip Jagodzinski added 3 commits January 3, 2020 12:11
HAL: ResetReason: Add the get_capabilities function
57420fc 
Add the hal_reset_reason_get_capabilities() to ResetReason HAL API.
Add a default, weak implementation, that every target can override.
Tests: ResetReason: Use the get_capabilities()
0b8baa5 
Make use of the new hal_reset_reason_get_capabilities() function to skip
unsupported reset resaons during tests.
K64F: Add hal_reset_reason_get_capabilities()
ffd8d70
@fkjagodzinski fkjagodzinski force-pushed the hal-reset_reason-get_capabilities branch from 5ac67eb to ffd8d70 Compare January 3, 2020 11:56
@fkjagodzinski
Copy link
Member Author

Before I approve, what other targets/families should implement this (at least to check if weakly defined is good for them) ?

This should be OK for all the targets that implement the reset reason API and had passed the tests.

Checking the hal_reset_reason_get() implementations, I can see that a few Toshiba targets are different and only provide RESET_REASON_POWER_ON, RESET_REASON_MULTIPLE and RESET_REASON_UNKNOWN. I'll add a hal_reset_reason_get_capabilities() for those.

TOSHIBA: Add hal_reset_reason_get_capabilities()
a958199 
Override the default, weak implementation of
hal_reset_reason_get_capabilities() for TMPM066, TMPM3H6, TMPM3HQ,
TMPM46B & TMPM4G9.
@fkjagodzinski
Copy link
Member Author

Implemented the hal_reset_reason_get_capabilities() for a few Toshiba targets that do not match the weak implementation.

@adbridge
Copy link
Contributor

adbridge commented Jan 6, 2020

@0xc0170 please review the updates

@0xc0170
Copy link
Contributor

0xc0170 commented Jan 16, 2020

CI started

@mbed-ci
Copy link

mbed-ci commented Jan 16, 2020

Test run: SUCCESS

Summary: 11 of 11 test jobs passed
Build number : 1
Build artifacts

@0xc0170 0xc0170 added release-version: 6.0.0-alpha-2 Second pre-release version of 6.0.0 and removed needs: work release-version: 6.0.0-alpha-1 First pre-release version of 6.0.0 labels Jan 16, 2020
@0xc0170 0xc0170 merged commit 31988d8 into ARMmbed:master Jan 16, 2020
@fkjagodzinski fkjagodzinski deleted the hal-reset_reason-get_capabilities branch January 16, 2020 14:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@0xc0170 0xc0170 0xc0170 approved these changes

@jamesbeyond jamesbeyond Awaiting requested review from jamesbeyond

@maclobdell maclobdell Awaiting requested review from maclobdell

+1 more reviewer

@mprse mprse mprse approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

release-version: 6.0.0-alpha-2 Second pre-release version of 6.0.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Issue with lack of hardware support causing mbed_hal-reset_reason test failure

7 participants

@fkjagodzinski @ciarmcom @adbridge @dustin-crossman @0xc0170 @mbed-ci @mprse