apache / mynewt-core

An OS to build, deploy and securely manage billions of devices
https://mynewt.apache.org/
Apache License 2.0
813 stars 365 forks source link

nRF52 syscfg.restrictions bug #3273

Open Uks2 opened 6 days ago

Uks2 commented 6 days ago

The nRF52 syscfg (mcu/nordic/nrf52xxx/syscfg.yml) has a load of restrictions on it serial peripherals, e.g. "!I2C_0 || (I2C_0_PIN_SCL && I2C_0_PIN_SDA)", aiming, presumably, the make sure that any enabled peripheral has it required pins configured.

It seems though that the settings are being evaluated numerically, i.e. the default empty string is true, but a value of 0 is false. This means that this setting passes through fine:

syscfg.vals:
    I2C_0: 1
    # I2C_0_PIN_SCL: "" (default)
    # I2C_0_PIN_SDA: "" (default)

but this will generate an error:

syscfg.vals:
    I2C_0: 1
    I2C_0_PIN_SCL: 0
    I2C_0_PIN_SDA: 1

I've tried changing the conditions to "... I2C_0_PIN_SCL != \"\" ..." with various different quote characters / methods of escaping, but couldn't find anything that tested the condition properly. So someone who knows a bit more about newt internals might have to take a look at it.

Thanks!

m-gorecki commented 3 days ago

Hi, settings evaluation does not look exactly the way you've described it. Empty string should evaluate to false and the reason that it seems to evaluate to true in your case, is probably that syscfg from bsp overrides it to some proper value.

However there is still one problem - as you've noticed pin number 0 is evaluated to false. This is indeed the case and it is problematic. Changing conditions to != \"\" will fix this, but it does not seem like an ideal solution. For now I've created a draft pull request with this kind of workaround: https://github.com/apache/mynewt-core/pull/3279

Uks2 commented 3 days ago

You're right, turns out I was defining it in both my bsp and my program. The 0 is false problem still stands though.

I've noticed that the nRF91 syscfg has all its pin settings default to -1, then does != -1 which seems marginally more elegant.

If you find something you like, it might be worth glancing some of the other BSPs; the nRF5340 one only does these checks for its UART settings, which doesn't seem right.