Skip to content
This repository has been archived by the owner on Dec 8, 2022. It is now read-only.

Fix nordic freertos timer create #2020

Merged
merged 3 commits into from
May 21, 2020

Conversation

ravibhagavandas
Copy link
Contributor

@ravibhagavandas ravibhagavandas commented May 18, 2020

Fix freertos timer create function for Nordic

Description

Nordic has an implementation for app timer APIs using FreeRTOS timers. app_timer_create API is expected to re-create a new timer with a new handler (note: there is no API to delete a timer). Currently the API returns an error if a timer already exists. With this PR, API ensures if timer is inactive, then delete and create a new timer with the new handler.

Also added a check in demo runner to handle user initiated network disable transition in network manager callback.

Checklist:

  • I have tested my changes. No regression in existing tests.
  • My code is Linted.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@ravibhagavandas
Copy link
Contributor Author

/bot run checks

qiutongs
qiutongs previously approved these changes May 18, 2020
@ravibhagavandas
Copy link
Contributor Author

/bot run checks

1 similar comment
@ravibhagavandas
Copy link
Contributor Author

/bot run checks

@ravibhagavandas
Copy link
Contributor Author

Note: Same issue is posted in Nordic forum and the fix is pending in Nordic SDK.
https://devzone.nordicsemi.com/f/nordic-q-a/57834/reenable-ble-after-disabling-fails-in-ble_conn_params_init-on-freertos

Fixing for now in the porting layer, until a proper fix has been merged from upstream.

@ravibhagavandas ravibhagavandas requested a review from gkwicker May 21, 2020 19:07
Copy link
Contributor

@gkwicker gkwicker left a comment

Choose a reason for hiding this comment

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

See comments

@@ -133,27 +134,35 @@ uint32_t app_timer_create(app_timer_id_t const * p_timer_id,
return NRF_ERROR_INVALID_STATE;
}

if (pinfo->osHandle == NULL)
/* If already a FreeRTOS timer exists, delete and recreate the timer. */
Copy link
Contributor

Choose a reason for hiding this comment

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

Grammar suggestion: "If a FreeRTOS timer already exists,..."

if (pinfo->osHandle == NULL)
err_code = NRF_ERROR_NULL;
}
if (mode == APP_TIMER_MODE_SINGLE_SHOT)
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add {...}, also lines 158-160

pinfo->func = timeout_handler;
pinfo->osHandle = xTimerCreate(" ", 1000, timer_mode, pinfo, app_timer_callback);

if (pinfo->osHandle == NULL)
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add {...}

@ravibhagavandas
Copy link
Contributor Author

/bot run checks

@ravibhagavandas ravibhagavandas merged commit a446766 into aws:master May 21, 2020
@ravibhagavandas ravibhagavandas deleted the fix/nordic_ble branch May 21, 2020 20:33
alfred2g pushed a commit to alfred2g/amazon-freertos that referenced this pull request May 21, 2020
* Fix nordic freertos timer create function

* Improving comments. include {}
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants