Comments from IRC

Development archive
#1

Hi,

the last few days there were two guys on IRC reviewing Riot for usability for a project of theirs. In the end sadly they deemed it too unfinished. On the other hand one of them was kind enough to give some input on what he deems amiss. I assured him that I'd relay it to the right places for discussion:

2014-10-11 11:41:37 corecode Hodapp: i didn't find anything 2014-10-11 13:49:26 Hodapp corecode: hrm, could one use the 32 kHz clock that needs to be running anyhow for the softdevice? 2014-10-11 15:16:20 corecode Hodapp: what do you mean by use? 2014-10-11 15:16:25 corecode you can use RTC1 2014-10-11 15:27:25 corecode Hodapp: but i don't think i'll use riot 2014-10-11 15:28:35 corecode Hodapp: i've come across several bugs and odd design decisions that seem to originate from the code not being used in production 2014-10-11 15:29:01 corecode like the rtt api... only one provider, no consumers, unclear what ticks even mean 2014-10-11 15:29:38 corecode Hodapp: we're looking at freertos now 2014-10-11 15:33:29 +temmi_hoo corecode: could you write an article of your findings to the riot-developers mailing list? 2014-10-11 15:35:49 corecode not particularly interested 2014-10-11 15:36:02 corecode it's a lot of small things 2014-10-11 15:36:23 corecode i can dump it here, and somebody else can initiate some discussion 2014-10-11 15:41:43 @N8Fear_ corecode: I think any form of input would be appreciated 2014-10-11 15:45:11 corecode ok 2014-10-11 15:45:28 corecode my input is: stuff is not tested 2014-10-11 15:45:43 corecode people seem to create APIs without applications that actually need them 2014-10-11 15:55:06 @N8Fear_ you said you also found some bugs: would you care to share what exactly? 2014-10-11 15:58:44 corecode use of dINT/eINT or disableIRQ/enableIRQ instead of restoreIRQ 2014-10-11 15:59:34 corecode the nrf timer code does not seem to be right/untested 2014-10-11 16:00:04 corecode why do boards define soc peripherals, e.g. timers 2014-10-11 16:00:57 corecode vtimer assumes 4096 seconds is a valid hwtimer value 2014-10-11 16:02:19 corecode why does some code use both hwtimer and vtimer? 2014-10-11 16:02:32 corecode what's the deal with if (!inISR()){ dINT();} 2014-10-11 16:02:39 corecode etc.etc. 2014-10-11 16:02:52 corecode and that's just after one day of looking at the source 2014-10-11 16:03:02 corecode why is there immense copy+pasteness happening? 2014-10-11 16:03:17 corecode hwtimer arch is copied all over the place 2014-10-11 16:03:59 corecode it mostly looks like an effort to "support" many platforms 2014-10-11 16:28:14 corecode N8Fear_: will you forward these points to the relevant places? 2014-10-11 16:28:30 @N8Fear_ sure 2014-10-11 16:31:48 @N8Fear_ otherwise I shouldn't have asked for your input. :wink: 2014-10-11 16:31:53 @N8Fear_ thank you! 2014-10-11 18:31:37 corecode my pleasure

Maybe we could take a look at the mentioned topics and see if we agree that there are issue and - in that case resolve them - or discard them as non-issues to us.

WKR Hinnerk

#2

Hi,

sad to hear...

2014-10-11 15:45:28 corecode my input is: stuff is not tested

I think we need to differentiate here, some platform code is indeed not sufficiently tested (yet). Maybe we can include some information about maturity of platforms in the Wiki?

2014-10-11 15:45:43 corecode people seem to create APIs without applications that actually need them

I guess this is a hen-and-egg problem, you don't get people to use stuff if the APIs they need are missing. But I also think, that most of our interfaces are designed quite well and should fit most applications. But this needs of course to be discussed continuously to be improved!

2014-10-11 15:55:06 @N8Fear_ you said you also found some bugs: would you care to share what exactly? 2014-10-11 15:58:44 corecode use of dINT/eINT or disableIRQ/enableIRQ instead of restoreIRQ

This is a difficult topic, there are some situations where you explicitly have to use 'enableIRQ' instead of 'restoreIRQ'. I think there are also open PRs to this issue already.

2014-10-11 15:59:34 corecode the nrf timer code does not seem to be right/untested

true. It's very young code so I wouldn't be surprised. There is one known bug with the vtimer.

2014-10-11 16:00:04 corecode why do boards define soc peripherals, e.g. timers

because they have an (passive) influence on the pin layout. The timers you choose for example for hwtimer and for PWM are coupled, and this has an impact of the pins you use for PWM. So the rationale is to make everything about peripherals configurable in a central point for each board.

2014-10-11 16:00:57 corecode vtimer assumes 4096 seconds is a valid hwtimer value

no idea, anyone?

2014-10-11 16:02:19 corecode why does some code use both hwtimer and vtimer?

some of the usages are for legacy reasons. Most of the usages are however following a straight forward paradim: 'user space' application code should use the vtimer, device drivers can use the hwtimer if they have strict real-time requirements.

2014-10-11 16:02:32 corecode what's the deal with if (!inISR()){ dINT();}

legacy code. There is an open task to finally remove these in favor of enableIRQ and disableIRQ.

2014-10-11 16:02:39 corecode etc.etc. 2014-10-11 16:02:52 corecode and that's just after one day of looking at the source 2014-10-11 16:03:02 corecode why is there immense copy+pasteness happening? 2014-10-11 16:03:17 corecode hwtimer arch is copied all over the place

For a good reason. There are some boards, that use more then one peripheral timer for the hwtimer. By having a per-CPU hwtimer_arch implementation each CPU is completely independent and can handle this as needed. Of course there is some code-duplication, but I think its a small price to pay for gaining the decoupling of CPU implementations.

2014-10-11 16:03:59 corecode it mostly looks like an effort to "support" many platforms

correct. This is one of the more challenging aspects of RIOT.

I hope I could clarify some points and I look forward to a good discussion!

Cheers, Hauke

#3

Hi!

>2014-10-11 15:45:28 corecode my input is: stuff is not tested I think we need to differentiate here, some platform code is indeed not sufficiently tested (yet). Maybe we can include some information about maturity of platforms in the Wiki?

Agreed. I think that's planned for a long time anyway.

>2014-10-11 15:58:44 corecode use of dINT/eINT or disableIRQ/enableIRQ instead of restoreIRQ This is a difficult topic, there are some situations where you explicitly have to use 'enableIRQ' instead of 'restoreIRQ'. I think there are also open PRs to this issue already.

There's an issue and PRs to fix some usage of the legacy API: https://github.com/RIOT-OS/RIOT/issues/197 https://github.com/RIOT-OS/RIOT/pull/922 https://github.com/RIOT-OS/RIOT/pull/1100 (merged)

>2014-10-11 16:00:57 corecode vtimer assumes 4096 seconds is a valid hwtimer value no idea, anyone?

Because hwtimer uses 32 bit timestamps. As long as the underlying timer does not run faster as 1MHz, that sounds like a valid assumption.

Cheers, Oleg

#4

let's start a FAQ wiki page and drop all the questions and answers from IRC/mailing list there.

Christian

#5

Good idea!

Cheers, Ludwig

#6

Hi,

#7

Hi Ludwig!

> > >2014-10-11 16:00:57 corecode vtimer assumes 4096 seconds is a valid hwtimer value > > no idea, anyone? > > Because hwtimer uses 32 bit timestamps. As long as the underlying timer does > not run faster as 1MHz, that sounds like a valid assumption.

But this sounds like it does make sense to have it configurable.

I don't understand exactly what should be configurable, but I agree that we should aim for a different timer architecture/concept for the new release (after the fall 2014 release).

Btw: Didn't a related issue also come up in @rousselk's recent timer PR?

Which one?

Cheers, Oleg