Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merge syscfg.yml sections with the same name #198

Open
andrzej-kaczmarek opened this issue Aug 7, 2018 · 4 comments
Open

Merge syscfg.yml sections with the same name #198

andrzej-kaczmarek opened this issue Aug 7, 2018 · 4 comments

Comments

@andrzej-kaczmarek
Copy link
Contributor

andrzej-kaczmarek commented Aug 7, 2018

Would be nice if newt could merge multiple sections with the same name in single .yml file instead of just using last one, especially syscfg.defs and syscfg.vals. This could be useful in large configurations for BSPs or apps where many different features are (re)defined so splitting these settings into groups would improve readability.

For example, I'd like to have clean up syscfg.yml a bit and do something like this:

# configure pins
syscfg.vals:
    PIN1: 1
    PIN2: 2
syscfg.vals.FEATURE_1:
    PIN7: 7
    PIN8: 8

# configure some other settings
syscfg.vals:
   FOO: 'bar'

Currently it seems that 2nd syscfg.vals overwrites everything defined in 1st syscfg.vals (and the same for syscfg.defs)

@ccollins476ad
Copy link
Contributor

This already works for me. I created a target called targets/mytarget with the following syscfg:

syscfg.vals:
    LIS2DW12_CLI: 5

syscfg.vals.LIS2DW12_INT1_CFG_ACTIVE:
    LIS2DW12_INT1_PIN_DEVICE: 2
    LIS2DW12_INT1_PIN_HOST: 2

syscfg.defs:
    GAGA:
        value: 1

syscfg.defs.LIS2DW12_INT1_CFG_ACTIVE:
    BABA:
        value: 1

When I view the config for this target, I see (truncated):

[@apache-mynewt-core/hw/drivers/sensors/lis2dw12]
  LIS2DW12_CLI: 5 (overridden by targets/mytarget)
  LIS2DW12_INT1_PIN_DEVICE: 2 (overridden by targets/mytarget)
  LIS2DW12_INT1_PIN_HOST: 2 (overridden by targets/mytarget)

[targets/mytarget]
  BABA: 1
  GAGA: 1

So the bug you saw might be more subtle. Could you please paste the output of newt target config brief <target> (or send it to me if it is private)? Thanks!

@andrzej-kaczmarek
Copy link
Contributor Author

Your syscfg does not have sections with the same name :-)

What I meant is as follows:

syscfg.vals:
        UART_0: 0
syscfg.vals.BLE_DEVICE:
        UART_1: 1
syscfg.vals:
        UART_1_PIN_TX: 10
        UART_1_PIN_RX: 11

In this case UART_0 is not overriden because only 2nd syscfg.vals section is used:

[@apache-mynewt-core/hw/mcu/nordic/nrf52xxx]
  UART_0: 1
  UART_0_PIN_CTS: 7 (overridden by @apache-mynewt-core/hw/bsp/nrf52840pdk)
  UART_0_PIN_RTS: 5 (overridden by @apache-mynewt-core/hw/bsp/nrf52840pdk)
  UART_0_PIN_RX: 8 (overridden by @apache-mynewt-core/hw/bsp/nrf52840pdk)
  UART_0_PIN_TX: 6 (overridden by @apache-mynewt-core/hw/bsp/nrf52840pdk)
  UART_1: 1 (overridden by targets/btshell)
  UART_1_PIN_CTS: -1
  UART_1_PIN_RTS: -1
  UART_1_PIN_RX: 11 (overridden by targets/btshell)
  UART_1_PIN_TX: 10 (overridden by targets/btshell)

Also it does not show up in target config:

targets/btshell
    app=@apache-mynewt-nimble/apps/btshell
    bsp=@apache-mynewt-core/hw/bsp/nrf52840pdk
    build_profile=debug
    syscfg=UART_1_PIN_RX=11:UART_1_PIN_TX=10

It would be nice if settings from both syscfg.vals sections are included (and also, I think, the same should apply to conditional sections)

@ccollins476ad
Copy link
Contributor

ccollins476ad commented Aug 9, 2018

I see what you mean. Not sure how I missed that in my initial reading! I do agree this would be an improvement. Taking the union of all these mappings, rather than replacing earlier definitions, eliminates some ambiguity

This will require a change to the YAML parsing library. From a YAML-standards perspective, I think this change is OK. From my reading, I believe this is all the YAML specification has to say on the subject (http://yaml.org/spec/1.2/spec.html, 3.2.1.1):

The content of a mapping node is an unordered set of key: value node pairs, with the restriction that each of the keys is unique. YAML places no further restrictions on the nodes.

So, it sounds like duplicate keys are undefined; the implementation can use its own discretion when it encounters them.

@andrzej-kaczmarek
Copy link
Contributor Author

I'm not familiar with YAML spec but googled a bit and it seems that duplicated keys are not allowed - I don't follow concepts in YAML spec so can't really argue here :-)

Anyway, perhaps to make things simpler here and avoid changing YAML parser the same thing could be done with new key which define sections in file and settings from each sections would be then merged in "post processing", something like this:

syscfg.vals:
        UART_0: 0
syscfg.vals.BLE_DEVICE:
        UART_1: 1

syscfg.section.my_section_with_extra_settings:
    syscfg.vals:
            UART_1_PIN_TX: 10
            UART_1_PIN_RX: 11

This can be read with standard YAML parser and then settings from all sections would be merged with the same settings in "default" section. But I do not know if this makes implementation easier in any way as I did not dig into process of reading syscfg files in newt so just posting this as another proposal for discussion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants