drivers: counter: Add user_data to alarm callback #11572

nordic-krch · 2018-11-21T12:59:22Z

Modify alarm callback to return user_data and channel_id.
Set_alarm and disable_alarm updated accordingly.

Updated counter.h

Nordic drivers and test aligned.

Signed-off-by: Krzysztof Chruscinski [email protected]

nordic-krch · 2018-11-21T13:07:08Z

fyi: @MaureenHelm @fvincenzo @SavinayDharmappa @stanislav-poboril

codecov-io · 2018-11-21T14:20:19Z

Codecov Report

Merging #11572 into topic-counters will not change coverage.
The diff coverage is n/a.

@@               Coverage Diff               @@
##           topic-counters   #11572   +/-   ##
===============================================
  Coverage           53.94%   53.94%           
===============================================
  Files                 242      242           
  Lines               27654    27654           
  Branches             6717     6717           
===============================================
  Hits                14917    14917           
  Misses               9932     9932           
  Partials             2805     2805

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f81e4d7...fdd8849. Read the comment docs.

mnkp

Looks good, with some minor comments. Also Shippable needs to be made happy.

include/counter.h

nordic-krch · 2018-11-27T12:45:24Z

Added renaming counter_set_ch_alarm to counter_set_channel_alarm.

@anangl suggested offline that dev could also be removed from counter_wrap_callback_t. This way it will be consistent with alarm callback. However, there is a backward compatibility issue. Once device is removed from wrap callback there is no straightforward way for having backward compatibility (counter_set_alarm) so I'm not sure what to do.
If we favor more consistent API over backward compatibility, then we can add optional backward compatibility to the driver (kconfig option) which would provide callback(dev, user_data). It can even be runtime per instance. What do you think? @nashif any practices you already took for cases like that?

MaureenHelm · 2018-11-28T01:48:50Z

include/counter.h

 */
-typedef void (*counter_alarm_callback_t)(struct device *dev,
-					 const struct counter_alarm_cfg *cfg,
+typedef void (*counter_alarm_callback_t)(void *user_data, u8_t chan_id,


I commented in #8340 (comment) that I don't agree with removing the dev argument to the callback.

@MaureenHelm can you please summarize how to implement more advanced use cases like the one described in #8340 (comment) without making this change?

See #11976 for rationale for providing both dev and a generic pointer.

After reading #11976 and re-thinking this particular case, I think we should keep the dev pointer and add the generic one as the additional parameter. Apart from the semi-consistency argument raised by @MaureenHelm (see #8340 (comment)) I can imagine cases where using a common user_data for multiple devices would be desirable and then having only one generic pointer would force the users to use an additional structure for just holding both the dev and user_data pointers. This does not seem to be reasonable. Besides, the dev pointer "comes for free" (we'll almost always have it at hand in the routine that will call the callback, no need to use additional storage for it) so just passing it as the first parameter will not be a big cost.

I can imagine cases where using a common user_data for multiple devices would be desirable and then having only one generic pointer would force the users to use an additional structure for just holding both the dev and user_data pointers.

The GPIO case described in #11976 is a bit different. Had GPIO callback passed a generic user_data pointer as a parameter there would be no problem getting access to all the required data.

Besides, the dev pointer "comes for free"

It does come cheap. But - at least on ARM architecture - only up to 4 parameters are passed to the function as register variables. If the parameter list is longer we need to put them on the stack. So if we decide to have a general convention to always pass pointer to dev and to user_data there is a price to pay.

So far I haven't seen anywhere in the Zephyr's code (or an application) a place where having a pointer to user_data passed in the callback function would turn out to be insufficient. If someone could point to an example that proves to the contrary that certainly would be major argument for passing both parameters.

I don't think there's any argument that it isn't technically sufficient to pass only user_data. It's just that in some cases that requires the user of the API to do extra work to capture the desired non-device pointer and the device pointer into an persistent object that wouldn't be needed if the handler were given both values. It's not quite "no problem".

Sorry, by "would turn out to be insufficient" I actually meant

requires the user of the API to do extra work to capture the desired non-device pointer and the device pointer into an persistent object that wouldn't be needed if the handler were given both values.

I really haven't seen it. Could you point me to a place in the code where such plumbing was necessary? In the GPIO case the callback doesn't pass user_data so the example is not adequate.

Replied here since that's where we're discussing the general case. I don't know enough about the counter API to create an equivalent situation, though I'm pretty sure it could be done.

include/counter.h

MaureenHelm · 2018-11-28T01:51:27Z

include/counter.h

-typedef int (*counter_api_set_alarm)(struct device *dev,
-				const struct counter_alarm_cfg *alarm_cfg);
-typedef int (*counter_api_disable_alarm)(struct device *dev,
+typedef int (*counter_api_set_alarm)(struct device *dev, u8_t chan_id,


Does the the application need to request a specific channel id? Could we instead return a channel id like the watchdog api?

then driver must implement some kind of allocator. In case of single instance watchdog which is shared between multiple subsystems that's good approach but in case of counter it will most likely be used by single subsystem or object so allocator, imo, is unnecessary complication of the driver.

include/counter.h

MaureenHelm · 2018-11-28T01:55:02Z

tests/drivers/counter/counter_basic_api/src/test_counter.c

@@ -148,11 +145,11 @@ void test_single_shot_alarm_instance(const char *dev_name)
 	err = counter_set_wrap(dev, ticks, wrap_handler, exp_user_data);
 	zassert_equal(0, err, "Counter failed to set wrap\n");

-	err = counter_set_alarm(dev, &alarm_cfg);
+	err = counter_set_ch_alarm(dev, 0, &alarm_cfg);


Aha! Like you said in the api meeting today, you did change the test to use counter_set_ch_alarm. It just isn't in the topic branch yet.

MaureenHelm · 2018-11-28T02:00:14Z

include/counter.h

@@ -75,8 +75,7 @@ struct counter_alarm_cfg {
 typedef void (*counter_wrap_callback_t)(struct device *dev, void *user_data);

 /* Deprecated counter callback. */
-typedef void (*counter_callback_t)(struct device *dev,
-						void *user_data);
+typedef void (*counter_callback_t)(struct device *dev, void *user_data);


Why are you changing the deprecated callback in this commit? It seems like an unrelated change to the renaming of counter_set_ch_alarm to counter_set_channel_alarm

yep, i will clean up a bit this PR since it became aggregation of couple of changes. Will split them into separate PRs

extracted it to independent commit

mnkp · 2018-11-29T10:51:30Z

@anangl suggested offline that dev could also be removed from counter_wrap_callback_t. This way it will be consistent with alarm callback. However, there is a backward compatibility issue. Once device is removed from wrap callback there is no straightforward way for having backward compatibility (counter_set_alarm) so I'm not sure what to do.
If we favor more consistent API over backward compatibility, then we can add optional backward compatibility to the driver (kconfig option) which would provide callback(dev, user_data). It can even be runtime per instance.

One extra vote for a consistent API. Since we seem to agree that passing dev and user_data in the same call is redundant let's remove dev from counter_wrap_callback_t too.

As far as I know there is no official 'best practices' document on how to do API changes, even though there were talks on creating one. The preferred way however is to use __deprecated attribute. We did once an SPI API change via a Kconfig option where user could select old or new API and that turned out to be a bit cumbersome, some people were not happy about it. One disadvantage of using __deprecated is a need to come up with new function names like counter_set_channel_alarm even though the old names like counter_set_alarm are much better.

In case of counter API we can use __deprecated and keep API backward compatible if we stay with

__deprecated static inline int counter_set_alarm(struct device *dev,
				    counter_callback_t callback,
				    u32_t count, void *user_data)
{
	const struct counter_driver_api *api = dev->driver_api;

	return api->set_alarm(dev, callback, count, user_data);
}

rather than doing

	return counter_set_wrap(dev, count, callback, user_data);

since that's where all the problems come from.

nordic-krch · 2018-11-29T12:02:28Z

@mnkp, issue is with legacy callback having dev and user_data and by removing dev from counter callbacks we have no option to pass both using simple wrapper. That's why i've proposed to extend driver implementation with backward compatibility extension. I've made a draft of that (nordic-krch@e6eafe4).

@anangl , @MaureenHelm what do you think about that approach (of course, given that we remove dev from the callback parameter list)

mnkp · 2018-11-29T14:26:02Z

@nordic-krch my idea about solving backward compatibility issue was a bit different. I'll explain it in a bit more detail.

At the moment Shippable fails with the message

/home/buildslave/src/github.com/zephyrproject-rtos/zephyr/drivers/counter/counter_qmsi_aonpt.c:154:15: error: initialization from incompatible pointer type [-Werror=incompatible-pointer-types]
  .set_alarm = aon_timer_qmsi_set_alarm,
               ^~~~~~~~~~~~~~~~~~~~~~~~
/home/buildslave/src/github.com/zephyrproject-rtos/zephyr/drivers/counter/counter_qmsi_aonpt.c:154:15: note: (near initialization for aon_timer_qmsi_api.set_alarm)
cc1: all warnings being treated as errors
ninja: build stopped: subcommand failed.

that's because set_alarm in counter_qmsi_aonpt.c is implemented by aon_timer_qmsi_set_alarm which is still using the old counter_set_alarm signature.

Currently we do

static inline int counter_set_channel_alarm(struct device *dev, u8_t chan_id,
				      const struct counter_alarm_cfg *alarm_cfg)
{
	const struct counter_driver_api *api = dev->driver_api;

	if (chan_id >= counter_get_num_of_channels(dev)) {
		return -ENOTSUP;
	}

	return api->set_alarm(dev, chan_id, alarm_cfg);
}

and I believe we shouldn't.

I reckon it's better to keep the new implementation straightforward, that means:

static inline int counter_set_channel_alarm(struct device *dev, u8_t chan_id,
				      const struct counter_alarm_cfg *alarm_cfg)
{
	const struct counter_driver_api *api = dev->driver_api;

	if (chan_id >= counter_get_num_of_channels(dev)) {
		return -ENOTSUP;
	}

	return api->set_channel_alarm(dev, chan_id, alarm_cfg);
}

__deprecated static inline int counter_set_alarm(struct device *dev,
				    counter_callback_t callback,
				    u32_t count, void *user_data)
{
	const struct counter_driver_api *api = dev->driver_api;

	return api->set_alarm(dev, callback, count, user_data);
}

i.e. counter_set_channel_alarm should call api->set_channel_alarm and counter_set_alarm should call api->set_alarm. We leave the old implementation intact and simply add a new one. In this case using different callback signatures will not cause any problem, there will be also no need for 'COUNTER_DEPRECATED_API_SUPPORT' Kconfig option.

include/counter.h

SavinayDharmappa · 2018-12-13T12:32:19Z

tests/drivers/counter/counter_basic_api/src/test_counter.c

+{
+	struct device *dev;
+	int err;
+	int cnt;


@nordic-krch isn't cnt variable should be u32_t ? In case of aonpt it is causing overflow as aonpt is down counter and intial value of counter 0xffffffff.

yes, i've updated that in this PR.

nordic-krch · 2018-12-18T11:05:57Z

@mnkp @anangl @MaureenHelm I've added struct device *dev to alarm callback. Updated nordic implementations and test.

anangl · 2018-12-18T13:03:03Z

I think we should also rename the counter_count_up function because currently its name may suggest that it requests the counter to count up, not just checks the counting type.
What about counter_counts_up or counter_is_counting_up?
I am not sure if such renaming is worth a separate PR, maybe it could be done here?

nordic-krch · 2018-12-18T13:53:38Z

@anangl added that to this commit

mnkp · 2018-12-18T15:22:00Z

include/counter.h

-typedef int (*counter_api_set_alarm)(struct device *dev,
-				const struct counter_alarm_cfg *alarm_cfg);
-typedef int (*counter_api_disable_alarm)(struct device *dev,
+typedef int (*counter_api_set_alarm)(struct device *dev, u8_t chan_id,


We've changed the signature of set_alarm function (counter_api_set_alarm) but didn't update all the drivers to reflect the change. The shippable is failing with

/home/buildslave/src/github.com/zephyrproject-rtos/zephyr/drivers/counter/counter_qmsi_aonpt.c:154:15: error: initialization from incompatible pointer type [-Werror=incompatible-pointer-types] .set_alarm = aon_timer_qmsi_set_alarm, ^~~~~~~~~~~~~~~~~~~~~~~~ /home/buildslave/src/github.com/zephyrproject-rtos/zephyr/drivers/counter/counter_qmsi_aonpt.c:154:15: note: (near initialization for aon_timer_qmsi_api.set_alarm)

The counter_qmsi_aonpt implements the old signature

static int aon_timer_qmsi_set_alarm(struct device *dev, counter_callback_t callback, u32_t count, void *user_data)

PR that broke the CI tests was merged earlier but we still need to fix this issue.

@mnkp #9726 address the issue of CI broken.

mnkp · 2018-12-18T19:12:06Z

I have a few more comments regarding function names. I'm however also not sure if they should be handled in this PR.

We have counter_set_channel_alarm and its counterpart counter_disable_channel_alarm. Maybe we should rename the latter to counter_clear_channel_alarm. APIs typically use set/clear and enable/disable in pairs without mixing them.

Several functions use 'ticks' parameter to pass counter value. Ticks has an immediate connotation with kernel ticks and is therefore very confusing. IMHO the old 'count' name was much better. Also function names like counter_ticks_to_us will cause issues, especially if kernel will use counter API to provide actual kernel ticks.

We have a function counter_set_wrap. I'm not a native English speaker but the 'wrap' part somehow doesn't feel right. I've also never seen it used in the context of the counter. Not in an API, not in a datasheet. I understand it's a tricky one as our counter can count up or down so the 'reload_value' doesn't work. But what about counter_set_top_value, counter_set_max_value? These are names used in the datasheets and users are more familiar with their meaning.

nordic-krch · 2018-12-19T07:17:40Z

We have counter_set_channel_alarm and its counterpart counter_disable_channel_alarm. Maybe we should rename the latter to counter_clear_channel_alarm.

clear_channel_alarm may be confusing as it suggests as if some kind of alarm flag got cleaned. Disable explains exactly what happens. If we look for symmetry then maybe set_channel_alarm-> enable_channel_alarm would be better?

Ticks has an immediate connotation with kernel ticks and is therefore very confusing.

That's subjective. I don't have such connotations. If we change parameter to count then we must have counter_count_to_us. That is confusing.

But what about counter_set_top_value, counter_set_max_value

I agree that wrap is not perfect. On the other hand, set_wrap sets periodic alarm and set_top_value or set_max_value does not sound like they are doing anything more than setting a value. What about something like set/enable_main_alarm (with complementary channel_alarms)?

anangl · 2018-12-19T08:36:28Z

Disable explains exactly what happens. If we look for symmetry then maybe set_channel_alarm-> enable_channel_alarm would be better?

Yes, enable/disable_channel_alarm looks reasonable to me.

I agree that wrap is not perfect. On the other hand, set_wrap sets periodic alarm and set_top_value or set_max_value does not sound like they are doing anything more than setting a value. What about something like set/enable_main_alarm (with complementary channel_alarms)?

Well, set/enable_main_alarm would suggest that the setting of the alarm is the only action taken. And the main purpose of this function is to set the wrap value, the callback to be associated with reaching this value is optional.

mnkp · 2018-12-19T13:48:16Z

Yes, enable/disable_channel_alarm looks reasonable to me.

+1

I agree with comments from @anangl. The main purpose of the counter_set_wrap function is to set the reload value in the count down mode and the compare/top value in the count up mode. The possibility to trigger an alarm is more of a side effect.

I would like to insist on reconsidering usage of the 'ticks' name:

In realm of RTOS 'tick' has a very specific meaning.
In kernel a 'tick' happens when the system timer overflows, in our case 'tick' happens on every clock.
Taking into account that Zephyr is targeted at low power IoT devices it's expected that a good share of users will want to configure kernel in tick-less mode and use a low power counter to supply kernel clock. The counter API usage of 'tick' will be very confusing.

That said I agree with the following

If we change parameter to count then we must have counter_count_to_us. That is confusing.

I don't have a good alternative and having the function is definitely useful.

anangl · 2018-12-20T10:04:20Z

I would like to insist on reconsidering usage of the 'ticks' name:

In realm of RTOS 'tick' has a very specific meaning.

In kernel a 'tick' happens when the system timer overflows, in our case 'tick' happens on every clock.

Taking into account that Zephyr is targeted at low power IoT devices it's expected that a good share of users will want to configure kernel in tick-less mode and use a low power counter to supply kernel clock. The counter API usage of 'tick' will be very confusing.

I agree with @nordic-krch that this is a subjective thing if one connects ticks primarily with kernel. To me it feels natural to use the term tick also for counters and I personally don't find it confusing even in the context of an RTOS. But I don't really have a strong opinion about this.
@pizi-nordic You created #12068 and you'd want to use the counter API in the implementation of the system clock source that provides kernel ticks. What's your opinion in this topic, do you find this naming confusing?

pizi-nordic · 2018-12-20T10:19:17Z

@anangl, @mnkp: I had similar discussion with @andyross when he reworked timer subsystem of the Zephyr. By my understanding the nomenclature is following:

Hardware timers/counters are counting in CYCLES.
Kernel uses a different time unit, as on some architectures hardware counters are pretty fast and we would have to use 64-bit calculations to deal with reasonable timeouts (like 30s). This unit is named TICK and consist of integer number of CYCLES.
The timer interrupt announces how many TICKS passed from last announcement. As this is done only when needed, we could say that Zephyr is tickless (there is no regular clock ticks, announcement is scheduled when kernel needs a wake up signal).

So as you see the TICK in Zephyr (time unit) is much closer to "jiffy" in Linux (also time unit) than the classic tick definition (regular timer interrupt). And this is a bit confusing.

pabigot · 2018-12-20T13:26:53Z

In some uses the counter is incremented not by time but by an external signal. From that I'm used to thinking of the increment to a counter as being a "tick", even if the same word also describes the OS granularity for timeslicing. Using "cycles" for any counter other than something with a direct (probably 1-1) relation to the fundamental instruction issue rate or CPU frequency would be more confusing.

Absent a clean alternative my vote is: stick with tick.

please revisit

pabigot · 2019-01-21T14:12:18Z

@nordic-krch this one also needs to be converted to a cherry-pick of the one commit. Rebasing makes a mess. I'm not sure how the disable of power_test got into this branch either (well, I know "how", I don't know "why").

No rush, now that I know what's going on I'll cherry-pick the pieces I need.

Modify alarm callback to return user_data and channel_id. Set_alarm and disable_alarm updated accordingly. Renamed counter_*_ch_alarm to counter_*_channel_alarm. Updated test and nrf implementations. Updated doxygen comments. Signed-off-by: Krzysztof Chruscinski <[email protected]>

nordic-krch · 2019-01-22T17:31:02Z

proposed change of naming as suggested by @mnkp would look like:
counter_set_wrap -> counter_set_top_value
counter_get_wrap -> counter_get_top_value
counter_get_max_wrap -> counter_get_max_top_value

@mnkp @pabigot any counter-proposals?

Renamed: - counter_set_wrap to counter_set_top_value - counter_get_wrap to counter_get_top_value - counter_get_max_wrap to counter_get_max_top_value Updated nRF implementations and counter test. Signed-off-by: Krzysztof Chruscinski <[email protected]>

nordic-krch · 2019-01-23T07:21:46Z

@pabigot @mnkp wrap stuff renamed, please re-review.

pabigot · 2019-01-23T14:53:13Z

@nordic-krch I think it would be best to move the top rename out of this PR. Looking at it I'm noticing some things I think are problematic with the API and need to think about and describe, and I don't want to delay merging the callback rework since it's blocking other platforms from updating.

pabigot

Never mind waiting; of course this changes API that people have to update for too.

One non-blocking comment.

pabigot · 2019-01-23T16:00:21Z

drivers/counter/counter_nrfx_rtc.c

-	u32_t wrap;
+	counter_top_callback_t top_cb;
+	void *top_user_data;
+	u32_t top;


I would prefer this be top_value, but I won't block for that.

mnkp · 2019-01-25T15:49:50Z

It's a bit late but this PR is missing one change that we agreed to perform.

We have counter_set_channel_alarm and its counterpart counter_disable_channel_alarm. Maybe we should rename the latter to counter_clear_channel_alarm. APIs typically use set/clear and enable/disable in pairs without mixing them.

Disable explains exactly what happens. If we look for symmetry then maybe set_channel_alarm-> enable_channel_alarm would be better?

Yes, enable/disable_channel_alarm looks reasonable to me.

It would be good to align these names before topic-counters is merged.

nordic-krch · 2019-01-28T11:41:23Z

@mnkp yes, forgot about it. Lets wait with that until all remaining PR's (3 now) will go into topic-counters. It will be easier to change that at once on all implementations then forcing yet another round of aligning for each PR.

pabigot · 2019-01-28T11:54:30Z

@mnkp @nordic-krch Completely agree about being consistent, but....

Somewhere in an issue comment it was clarified that counter_set_channel_alarm configures a one-shot alarm that is not automatically repeated when the counter wraps. That's not yet in the documentation AFAICT, and maybe my recollection is wrong.

In my opinion if this is a one-shot alarm it should be set/cancel. If it's a repeating alarm it should be enable/disable.

Does this work for everybody?

nordic-krch requested review from galak, anangl and mnkp November 21, 2018 12:59

mnkp reviewed Nov 22, 2018

View reviewed changes

include/counter.h Outdated Show resolved Hide resolved

include/counter.h Outdated Show resolved Hide resolved

include/counter.h Outdated Show resolved Hide resolved

include/counter.h Show resolved Hide resolved

mnkp reviewed Nov 22, 2018

View reviewed changes

include/counter.h Outdated Show resolved Hide resolved

nordic-krch requested review from MaureenHelm and nashif November 27, 2018 17:13

MaureenHelm added area: Counter area: API Changes to public APIs labels Nov 28, 2018

MaureenHelm previously requested changes Nov 28, 2018

View reviewed changes

nordic-krch force-pushed the counter_callback_api branch from 46496c8 to c78e4a1 Compare November 29, 2018 11:26

mnkp reviewed Nov 29, 2018

View reviewed changes

include/counter.h Outdated Show resolved Hide resolved

nordic-krch force-pushed the topic-counters branch from 2e57cff to 215f872 Compare December 4, 2018 10:27

pabigot mentioned this pull request Dec 12, 2018

APIs that support a callback should provide both the device pointer and a generic pointer #11976

Closed

SavinayDharmappa reviewed Dec 13, 2018

View reviewed changes

nordic-krch force-pushed the counter_callback_api branch 3 times, most recently from a160175 to 9c24fa0 Compare December 18, 2018 11:04

nordic-krch force-pushed the counter_callback_api branch from 9c24fa0 to 71643fe Compare December 18, 2018 13:53

mnkp reviewed Dec 18, 2018

View reviewed changes

nashif force-pushed the topic-counters branch from 215f872 to 6b96890 Compare January 15, 2019 13:45

nashif force-pushed the counter_callback_api branch from 71643fe to 8962770 Compare January 15, 2019 21:22

nordic-krch force-pushed the topic-counters branch 2 times, most recently from 2f603c6 to 72f7261 Compare January 21, 2019 08:19

nordic-krch force-pushed the counter_callback_api branch from 5af3910 to 2766569 Compare January 22, 2019 06:58

nordic-krch mentioned this pull request Jan 22, 2019

Fixes for topic-counters branch #12634

Merged

nordic-krch force-pushed the counter_callback_api branch from 2766569 to c873027 Compare January 22, 2019 08:35

nordic-krch force-pushed the counter_callback_api branch from c873027 to 0ff4c78 Compare January 22, 2019 13:53

pabigot approved these changes Jan 22, 2019

View reviewed changes

pabigot approved these changes Jan 23, 2019

View reviewed changes

mnkp approved these changes Jan 23, 2019

View reviewed changes

nashif merged commit 6f5bf6a into zephyrproject-rtos:topic-counters Jan 23, 2019

stanislav-poboril mentioned this pull request Jan 24, 2019

i.MX EPIT Timer Driver #11441

Merged

drivers: counter: Add user_data to alarm callback #11572

drivers: counter: Add user_data to alarm callback #11572

Conversation

nordic-krch commented Nov 21, 2018

nordic-krch commented Nov 21, 2018

codecov-io commented Nov 21, 2018 • edited Loading

Codecov Report

mnkp left a comment

Choose a reason for hiding this comment

nordic-krch commented Nov 27, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mnkp commented Nov 29, 2018

nordic-krch commented Nov 29, 2018 • edited Loading

mnkp commented Nov 29, 2018 • edited Loading

SavinayDharmappa Dec 13, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nordic-krch commented Dec 18, 2018

anangl commented Dec 18, 2018

nordic-krch commented Dec 18, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mnkp commented Dec 18, 2018 • edited Loading

nordic-krch commented Dec 19, 2018

anangl commented Dec 19, 2018

mnkp commented Dec 19, 2018

anangl commented Dec 20, 2018

pizi-nordic commented Dec 20, 2018 • edited Loading

pabigot commented Dec 20, 2018

pabigot commented Jan 21, 2019

nordic-krch commented Jan 22, 2019

nordic-krch commented Jan 23, 2019

pabigot commented Jan 23, 2019

pabigot left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mnkp commented Jan 25, 2019

nordic-krch commented Jan 28, 2019

pabigot commented Jan 28, 2019

codecov-io commented Nov 21, 2018 •

edited

Loading

nordic-krch commented Nov 29, 2018 •

edited

Loading

mnkp commented Nov 29, 2018 •

edited

Loading

SavinayDharmappa Dec 13, 2018 •

edited

Loading

mnkp commented Dec 18, 2018 •

edited

Loading

pizi-nordic commented Dec 20, 2018 •

edited

Loading