Hi Sigve,
On Tue, Jun 06, 2017 at 01:01:20PM +0200, Sigve Sebastian Farstad wrote:
[...]
> So, there are perhaps two bugs in here. 1) why is MYNEWT_VAL_I2C_0 0 and
> not '0', as it is defined in syscfg.yml?
The single quotes are a YAML escaping mechanism similar to shell
quoting. So, values of '0' and 0 are identical here. The newt tool
just puts everything in single quotes to avoid potential issues with
special characters.
> And 2) I2C code is not included properly.
>
> For 1), CfgEntry structs in the newt tool's source only support string
> entries, so I imagine something gets when going from yml entries to C
> literals.
>
> One way of solving 2) is to use #ifdef instead of #if when checking if a
> value is defined. However, this doesn't work with the MYNEWT_VAL(..) macro,
> as far as I can tell, so one would need to use MYNEWT_VAL_I2C_0 directly
> instead.
>
> Currently, I've worked around the issue by setting the I2C_0 value in
> syscfg.yml to something truthy (e.g. true), but it seems to me that the
> intention is for it to be e.g. 0 for I2C_0 and 1 for I2C_1, since the code
> in hal_i2c.c seems to want to use these numbers as array indexes as well.
>
> Lastly, this probably affects more than just I2C as the #if MYNEWT_VAL(..)
> pattern is used many places in the core.
>
>
> Thoughts?
As others have indicated, this particular setting is meant to be a
boolean. A value of 0 means the I2C port is unused.
I do think you are right in the general case, though: there is a
potential ambiguity. It is not possible to test if a particular syscfg
setting is undefined:
1. If the package which defines a setting is not part of your project,
the corresponding MYNEWT_VAL_<...> macro won't be defined.
2. If an undefined macro is used in an `#if` context, it evaluates to 0.
So, the use of `#if` depends on the assumption that 0 and undefined are
equivalent in your code.
If you ever need to know if a MYNEWT_VAL setting is truly undefined, I
think you will need to bypass the MYNEWT_VAL() macro and test the value
directly with `#ifdef`, e.g.,
#ifdef MYNEWT_VAL_I2C_0
I don't think I have run into any cases where I've needed to distinguish
undefined from 0, though.
Chris
