Skip to content

[HAL] Add support for 64 bit us timestamp #4094

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.

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

Merged
merged 19 commits into from
Jun 2, 2017
Merged
Changes from 1 commit
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
ticker api: format code to meet mbed standards.
  • Loading branch information
pan- committed May 16, 2017
commit c20a1ccd4ffea1042b3f0c731ac458086a091e6e
38 changes: 25 additions & 13 deletions hal/mbed_ticker_api.c
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
#include <stdio.h>
#include <stdio.h>
#include <stddef.h>
#include "hal/ticker_api.h"
#include "platform/mbed_critical.h"
Expand All @@ -26,7 +26,8 @@ static void update_current_timestamp(const ticker_data_t *const ticker);
/*
* Initialize a ticker instance.
*/
static void initialize(const ticker_data_t *ticker) {
static void initialize(const ticker_data_t *ticker)
{
// return if the queue has already been initialized, in that case the
// interface used by the queue is already initialized.
if (ticker->queue->initialized) {
Expand All @@ -47,7 +48,8 @@ static void initialize(const ticker_data_t *ticker) {
/**
* Set the event handler function of a ticker instance.
*/
static void set_handler(const ticker_data_t *const ticker, ticker_event_handler handler) {
static void set_handler(const ticker_data_t *const ticker, ticker_event_handler handler)
{
ticker->queue->event_handler = handler;
}

Expand All @@ -63,7 +65,8 @@ static void set_handler(const ticker_data_t *const ticker, ticker_event_handler
* @param ref: The timestamp of reference.
* @param relative_timestamp: The timestamp to convert.
*/
static us_timestamp_t convert_relative_timestamp(us_timestamp_t ref, timestamp_t relative_timestamp) {
static us_timestamp_t convert_relative_timestamp(us_timestamp_t ref, timestamp_t relative_timestamp)
{
bool overflow = relative_timestamp < ((timestamp_t) ref) ? true : false;

us_timestamp_t result = (ref & ~((us_timestamp_t)UINT32_MAX)) | relative_timestamp;
Expand All @@ -77,7 +80,8 @@ static us_timestamp_t convert_relative_timestamp(us_timestamp_t ref, timestamp_t
/**
* update the current timestamp value of a ticker.
Copy link
Contributor

Choose a reason for hiding this comment

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

when we are supposed to call this ? Whats the requirement?

Copy link
Member Author

Choose a reason for hiding this comment

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

User is not supposed to call this function, it is private (static). It can be called whenever the us_timestamp_t held by a ticker_data_t has to be updated.

Copy link
Contributor

Choose a reason for hiding this comment

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

My question was from the developer view, once I am about to edit this file, I was not clear when this function is supposed to be called.

*/
static void update_current_timestamp(const ticker_data_t *const ticker) {
static void update_current_timestamp(const ticker_data_t *const ticker)
{
ticker->queue->timestamp = convert_relative_timestamp(
ticker->queue->timestamp,
ticker->interface->read()
Expand All @@ -91,7 +95,8 @@ static void update_current_timestamp(const ticker_data_t *const ticker) {
* interrupt will be set to MBED_TICKER_INTERRUPT_TIMESTAMP_MAX_DELTA us from now.
* Otherwise the interrupt will be set to head->timestamp - queue->timestamp us.
*/
static void update_interrupt(const ticker_data_t *const ticker) {
static void update_interrupt(const ticker_data_t *const ticker)
{
update_current_timestamp(ticker);
uint32_t diff = MBED_TICKER_INTERRUPT_TIMESTAMP_MAX_DELTA;

Expand All @@ -107,12 +112,14 @@ static void update_interrupt(const ticker_data_t *const ticker) {
);
}

void ticker_set_handler(const ticker_data_t *const ticker, ticker_event_handler handler) {
void ticker_set_handler(const ticker_data_t *const ticker, ticker_event_handler handler)
{
initialize(ticker);
set_handler(ticker, handler);
}

void ticker_irq_handler(const ticker_data_t *const ticker) {
void ticker_irq_handler(const ticker_data_t *const ticker)
{
ticker->interface->clear_interrupt();

/* Go through all the pending TimerEvents */
Expand Down Expand Up @@ -142,7 +149,8 @@ void ticker_irq_handler(const ticker_data_t *const ticker) {
update_interrupt(ticker);
}

void ticker_insert_event(const ticker_data_t *const ticker, ticker_event_t *obj, timestamp_t timestamp, uint32_t id) {
void ticker_insert_event(const ticker_data_t *const ticker, ticker_event_t *obj, timestamp_t timestamp, uint32_t id)
{
/* disable interrupts for the duration of the function */
core_util_critical_section_enter();

Expand All @@ -161,7 +169,8 @@ void ticker_insert_event(const ticker_data_t *const ticker, ticker_event_t *obj,
);
}

void ticker_insert_event_us(const ticker_data_t *const ticker, ticker_event_t *obj, us_timestamp_t timestamp, uint32_t id) {
void ticker_insert_event_us(const ticker_data_t *const ticker, ticker_event_t *obj, us_timestamp_t timestamp, uint32_t id)
{
/* disable interrupts for the duration of the function */
Copy link
Contributor

Choose a reason for hiding this comment

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

prefer to have a reason why you are disabling interrupts, its clear we enter/exit critical section. same for the comment above (update the current timestamp - function name is the same to a reader). A reader is interested why there's a need for critical section - what we are protecting here and why

Copy link
Member Author

Choose a reason for hiding this comment

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

Ticker API is thread safe and interrupt safe therefore access to the ticker has to be protected from external modifications.

Copy link
Contributor

Choose a reason for hiding this comment

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

I was just referring there that the comment either can be removed (obvious that theres critical enter/exit) or we specify a reason why its under critical section. same for the below (update the current time stamp - function name says it).

core_util_critical_section_enter();

Expand Down Expand Up @@ -207,7 +216,8 @@ void ticker_insert_event_us(const ticker_data_t *const ticker, ticker_event_t *o
core_util_critical_section_exit();
}

void ticker_remove_event(const ticker_data_t *const ticker, ticker_event_t *obj) {
void ticker_remove_event(const ticker_data_t *const ticker, ticker_event_t *obj)
{
core_util_critical_section_enter();

// remove this object from the list
Expand All @@ -230,11 +240,13 @@ void ticker_remove_event(const ticker_data_t *const ticker, ticker_event_t *obj)
core_util_critical_section_exit();
}

timestamp_t ticker_read(const ticker_data_t *const ticker) {
timestamp_t ticker_read(const ticker_data_t *const ticker)
{
return ticker_read_us(ticker);
}

us_timestamp_t ticker_read_us(const ticker_data_t *const ticker) {
us_timestamp_t ticker_read_us(const ticker_data_t *const ticker)
{
update_current_timestamp(ticker);
return ticker->queue->timestamp;
}
Expand Down