Re: [riot-devel] Progress of port to SODAQ Autonomo

[Thomas Eichinger]

All,

I am a little concerned the coding conventions getting too pedantic 
lately.


While I agree magic numbers should be avoided, I disagree to introduce 
#defines
for every single one of them as I do with introducing a static check for 
it.
(Using a #define for a number only used once in HW initialization for 
example

seems too much to me.)

I agree however with putting a comment with an explanation or data sheet 
reference

to each occurrence.
(Register initializations should use or'ed bit defines from vendor 
headers anyways.)
To me it is the duty of the maintainer reviewing the code to decide if 
it makes
sense to use a #define or a comment is enough. Thus I'd prefer a best 
practice

instead of a rule.

Best, Thomas

On 4 Jul 2016, at 9:20 CEST(+0200), Ludwig Knüpfer wrote:


On Mon, Jul 04, 2016 at 07:44:21AM +0200, Ludwig Knüpfer wrote:

Am 2. Juli 2016 13:21:19 MESZ, schrieb Kees Bakker :
While going through the code I notice that there are too many 
"magic"

constants. Hard coded numbers that are obvious for some, but not
obvious
for others. My advise: always try to use defines and add a comment
about
what constants mean. Or point to datasheet sections explaining the
constants.


Thank you for bringing this up.

I am uncertain if there is something that can be done about it in the 
existing code base, but at least we should find a way to prevent such 
issues in the future:


There is an "immediately" missing in the sentence above... I guess
there's quite a few of these ;)

Cheers,
Ludwig
___
devel mailing list
devel@riot-os.org
https://lists.riot-os.org/mailman/listinfo/devel

___
devel mailing list
devel@riot-os.org
https://lists.riot-os.org/mailman/listinfo/devel

Re: [riot-devel] Progress of port to SODAQ Autonomo

[Ludwig Knüpfer]

On Mon, Jul 04, 2016 at 07:44:21AM +0200, Ludwig Knüpfer wrote:
> Am 2. Juli 2016 13:21:19 MESZ, schrieb Kees Bakker :
> >While going through the code I notice that there are too many "magic"
> >constants. Hard coded numbers that are obvious for some, but not
> >obvious
> >for others. My advise: always try to use defines and add a comment
> >about
> >what constants mean. Or point to datasheet sections explaining the 
> >constants.
> 
> Thank you for bringing this up.
> 
> I am uncertain if there is something that can be done about it in the 
> existing code base, but at least we should find a way to prevent such issues 
> in the future:

There is an "immediately" missing in the sentence above... I guess
there's quite a few of these ;)

Cheers,
Ludwig
___
devel mailing list
devel@riot-os.org
https://lists.riot-os.org/mailman/listinfo/devel

[riot-devel] Progress of port to SODAQ Autonomo

[Kees Bakker]

Despite the ongoing discussion of how cpu/sam* should be configured
I am continuing with my effort to port RIOT to SODAQ Autonomo.

The changes are available in my add-sodaq-autonomo branch
at g...@github.com:keestux/RIOT.git

UART and I2C are working now, and so is xtimer. Next on my list is SPI.

For I2C there are a few changes in gpio that make the code simpler
and configuration easier to understand. (At least, that's what I think :-)

Coding.
While going through the code I notice that there are too many "magic"
constants. Hard coded numbers that are obvious for some, but not obvious
for others. My advise: always try to use defines and add a comment about
what constants mean. Or point to datasheet sections explaining the 
constants.


One example (not to blame, but to hopefully improve in the future).

cpu/samd21/cpu.c:
/* redirect all peripherals to a disabled clock generator (7) by 
default */

for (int i = 0x3; i <= 0x22; i++) {
What is that 0x22? It probably is SAMR21's GCLK_PTC, the last non-reserved
Generic Clock Selection ID. BTW, the last non-reserved ID for SAMD21 is 0x24
which is GCLK_I2S_1.
This particular should probable disable all peripherals, even the 
reserved ones.

But the point is, the number 0x22 should explained.

--
Kees Bakker
Founder
SODAQ
M. 0031617737165
www.sodaq.com

___
devel mailing list
devel@riot-os.org
https://lists.riot-os.org/mailman/listinfo/devel