Skip to content

Conversation

gpsimenos
Copy link
Contributor

@gpsimenos gpsimenos commented Sep 11, 2019

Description

This PR is part of IOTCLOUDPR-3223. The requirement is to enable tickless mode in targets that CI can run tests on. In this first PR, tickless mode is enabled in MAX32630FTHR, which successfully passed all greentea tests with tickless mode.

An additional PR may follow with more targets that currently show minor greentea regressions, after it is confirmed tickless mode is not the cause.

Pull request type

[ ] Fix
[ ] Refactor
[X] Target update
[ ] Functionality change
[ ] Docs update
[ ] Test update
[ ] Breaking change

Reviewers

@evedon @hugueskamba

@ciarmcom
Copy link
Member

@gpsimenos, thank you for your changes.
@hugueskamba @evedon @ARMmbed/mbed-os-maintainers please review.

@ciarmcom ciarmcom requested review from evedon, hugueskamba and a team September 11, 2019 17:00
"PORTIN",
"PORTINOUT",
"PORTOUT",
"PWMOUT",
Copy link
Contributor

@0xc0170 0xc0170 Sep 12, 2019

Choose a reason for hiding this comment

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

commit this separately please - removing dead space

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey @0xc0170 did you want the removal of dead space in a separate PR or just a separate commit in this PR?

Copy link
Contributor

Choose a reason for hiding this comment

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

separate commit would work but as there are multiple commits, separate PR is easier.

Copy link
Collaborator

@hugueskamba hugueskamba left a comment

Choose a reason for hiding this comment

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

Please edit the first commit message to clearly list the targets for which tickless has been enabled.

Copy link
Collaborator

@hugueskamba hugueskamba left a comment

Choose a reason for hiding this comment

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

LGTM

@gpsimenos gpsimenos requested a review from 0xc0170 September 12, 2019 14:32
@evedon
Copy link
Contributor

evedon commented Sep 13, 2019

Changing release to 5.15 as this is a target change

@gpsimenos gpsimenos changed the title Enable tickless mode in successfully tested targets Enable tickless mode in MAX32630FTHR Sep 17, 2019
@gpsimenos gpsimenos changed the title Enable tickless mode in MAX32630FTHR Enable tickless mode in MAX32630FTHR and KL46Z Sep 17, 2019
Copy link
Contributor

@evedon evedon left a comment

Choose a reason for hiding this comment

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

Fine for the code change to enable tickless.
One comment left by Martin for the dead spaces.

@0xc0170
Copy link
Contributor

0xc0170 commented Sep 18, 2019

Ci started

@mbed-ci
Copy link

mbed-ci commented Sep 18, 2019

Test run: FAILED

Summary: 3 of 4 test jobs failed
Build number : 1
Build artifacts

Failed test jobs:

  • jenkins-ci/mbed-os-ci_build-ARM
  • jenkins-ci/mbed-os-ci_build-IAR
  • jenkins-ci/mbed-os-ci_build-GCC_ARM

@gpsimenos gpsimenos changed the title Enable tickless mode in MAX32630FTHR and KL46Z Enable tickless mode in MAX32630FTHR Sep 19, 2019
@gpsimenos
Copy link
Contributor Author

Last force push removes failing target.

@0xc0170
Copy link
Contributor

0xc0170 commented Sep 20, 2019

CI started

@mbed-ci
Copy link

mbed-ci commented Sep 20, 2019

Test run: SUCCESS

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

@0xc0170 0xc0170 merged commit 77dd6f0 into ARMmbed:master Sep 20, 2019
@adbridge adbridge added release-version: 6.0.0-alpha-1 First pre-release version of 6.0.0 and removed release-version: 5.15.0-rc1 labels Oct 3, 2019
@0xc0170 0xc0170 added release-version: 5.15.0-rc1 and removed release-version: 6.0.0-alpha-1 First pre-release version of 6.0.0 labels Nov 19, 2019
@idea--list
Copy link
Contributor

idea--list commented Feb 23, 2020

Hi there,
Opened issue ticket #12493 earlier this night.
After some hours of investigation i found in the release notes for 5.15.0 that tickless mode has been enabled in MAX32630FTHR. As it sounded suspicious i opened up targets.json, looked up the definitions for the MAX32630FTHR board and deleted the changes introduced by this merge.

Made a clean build and the problem described in #12493 is solved. If i undo the change in targets.json and clean build the code then the issue is back again.

It seems this merge has not been tested properly, please either undo this merge or solve the bug it introduces in crucial functions like wait(), wait_ms(), ThisThread::sleep_for(), etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants