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

Add a way to mutate JSON within manifests #85

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

puddly
Copy link
Collaborator

@puddly puddly commented Oct 22, 2024

See #84.

Example:

json_config:
  - file: "config/zcl/zcl_config.zap"
    jq: |
      .endpointTypes[].clusters[].attributes[] | select(.name == "model name") 
      | .defaultValue = "some model"'

  - file: "config/zcl/zcl_config.zap"
    jq: |
      .endpointTypes[].clusters[].attributes[] | select(.name == "manufacturer name") 
      | .defaultValue = "some manufacturer"'

tools/build_project.py Outdated Show resolved Hide resolved
tools/build_project.py Outdated Show resolved Hide resolved
@tl-sl
Copy link
Contributor

tl-sl commented Oct 23, 2024

This is what I have that is working (patch against this PR):
0001-patch-config-before-slc-generate.PATCH

manifest:

json_config:
  - file: "config/zcl/zcl_config.zap"
    jq: |
      ( .endpointTypes[].clusters[].attributes[] | select(.name == "model identifier"))
      .defaultValue = "some model"

  - file: "config/zcl/zcl_config.zap"
    jq: |
      ( .endpointTypes[].clusters[].attributes[] | select(.name == "manufacturer name"))
      .defaultValue = "some manufacturer"

@Nerivec
Copy link
Contributor

Nerivec commented Oct 29, 2024

My 2 cents on this:

  • Use a generic approach for values that always need 1) access to refs that python has, 2) to be set.
  • Use manifest-based for values specific to devices (probably could do some sort of mapping from yaml>python to avoid writing the whole jq commands in yaml, and just set values instead?).
  • Needs to be called before SLC, as Tim mentioned (LOGGER.info(f"Generating project for {manifest['device']}")).
  • Bit more logging
    zcl_config_zap = build_template_path / "config/zcl/zcl_config.zap"

    if zcl_config_zap.exists():
        date_code = datetime.today().strftime("%Y-%m-%d")
        default_zap_changes = [
            # Set software date code to day of build
            f'(.endpointTypes[].clusters[].attributes[] | select(.name == "date code")).defaultValue = "{date_code}"',
            # Set software build ID to SDK version
            f'(.endpointTypes[].clusters[].attributes[] | select(.name == "sw build id")).defaultValue = "{sdk_version.split(":", 1)[1]}"',
            # Set path
            f'(.package[] | select(.type == "zcl-properties")).path = "{sdk}/app/zcl/zcl-zap.json"',
            # Set path
            f'(.package[] | select(.type == "gen-templates-json")).path = "{sdk}/protocol/zigbee/app/framework/gen-template/gen-templates.json"',
        ]

        for zap_change in default_zap_changes:
            LOGGER.info("Default ZAP change: %s", zap_change)

            result = subprocess.run(
                [
                    "jq",
                    zap_change,
                    zcl_config_zap,
                ],
                capture_output=True,
            )

            if result.returncode != 0:
                LOGGER.error("jq stderr: %s\n%s", result.returncode, result.stderr)
                sys.exit(1)

            with open(zcl_config_zap, "wb") as f:
                f.write(result.stdout)

    # JSON config
    for json_config in manifest.get("json_config", []):
        json_path = build_template_path / json_config["file"]

        if not json_path.exists():
            raise ValueError(f"[{json_path}] does not exist")

        device_spe_change = json_config["jq"]

        LOGGER.info("Device-specific change in %s: %s", json_path, device_spe_change)

        result = subprocess.run(
            [
                "jq",
                device_spe_change,
                json_path,
            ],
            capture_output=True,
        )

        if result.returncode != 0:
            LOGGER.error("jq stderr: %s\n%s", result.returncode, result.stderr)
            sys.exit(1)

        with open(json_path, "wb") as f:
            f.write(result.stdout)

Note: This is tested and working on a Sonoff Dongle-E with router build. https://github.com/Nerivec/silabs-firmware-builder/actions/runs/11570015214/job/32204929046
Note2: sdk_version.split is already called above this, probably worth making a variable out of it

@Nerivec
Copy link
Contributor

Nerivec commented Oct 29, 2024

Since DEFAULT_JSON_CONFIG runs first, we can also set generic defaults for date code and sw build id, and then if a specific manifest wants to override it, it can, right?

@Nerivec
Copy link
Contributor

Nerivec commented Oct 30, 2024

Suggestion for cleaner manifests (can get pretty crowded if you want to keep the project generic enough):

DEFAULT_JSON_CONFIG = [
    # Fix a few paths by default
    {
        "file": "config/zcl/zcl_config.zap",
        "jq": '(.package[] | select(.type == "zcl-properties")).path = "template:{sdk}/app/zcl/zcl-zap.json"',
        "skip_if_missing": True,
    },
    {
        "file": "config/zcl/zcl_config.zap",
        "jq": '(.package[] | select(.type == "gen-templates-json")).path = "template:{sdk}/protocol/zigbee/app/framework/gen-template/gen-templates.json"',
        "skip_if_missing": True,
    },
    {
        "jq_func": 'zap_set_cluster_attribute("date code", "template:{today}", "all", 0, True)',
    },
    {
        "jq_func": 'zap_set_cluster_attribute("sw build id", "template:{sdk_version}", "all", 0, True)',
    },
]

def zap_select_endpoint_type(endpoint_type_id: int | str) -> str:
    return (
        ".endpointTypes[]"
        if endpoint_type_id == "all"
        else f".endpointTypes[] | select(.id == {endpoint_type_id})"
    )


def zap_select_cluster(cluster_code: int | str) -> str:
    return (
        ".clusters[]"
        if cluster_code == "all"
        else f".clusters[] | select(.code == {cluster_code})"
    )


def zap_delete_cluster(
    cluster_name: str,
    endpoint_type_id: int | str = "all",
    skip_if_missing: bool = False,
) -> list[dict[str, typing.Any]]:
    return [
        {
            "file": "config/zcl/zcl_config.zap",
            "jq": f'del({zap_select_endpoint_type(endpoint_type_id)}.clusters[] | select(.name == "{cluster_name}"))',
            "skip_if_missing": skip_if_missing,
        }
    ]


def zap_set_cluster_attribute(
    attribute_name: str,
    default_value: typing.Any,
    endpoint_type_id: int | str = "all",
    cluster_code: int | str = "all",
    skip_if_missing: bool = False,
) -> list[dict[str, typing.Any]]:
    return [
        {
            "file": "config/zcl/zcl_config.zap",
            "jq": f'({zap_select_endpoint_type(endpoint_type_id)}{zap_select_cluster(cluster_code)}.attributes[] | select(.name == "{attribute_name}")).defaultValue = "{default_value}"',
            "skip_if_missing": skip_if_missing,
        }
    ]
    # Template variables
    value_template_env = {
        "git_repo_hash": get_git_commit_id(repo=pathlib.Path(__file__).parent.parent),
        "manifest_name": args.manifest.stem,
        "now": datetime.now(timezone.utc),
        "today": date.today(),
        "sdk": sdk,
        "sdk_version": sdk_version,
    }

    LOGGER.info("Using templating env:")
    LOGGER.info(str(value_template_env))

    for json_config in DEFAULT_JSON_CONFIG + manifest.get("json_config", []):
        for json_config_item in (
            eval(expand_template(json_config["jq_func"], value_template_env))
            if json_config.get("jq_func", None)
            else [json_config]
        ):
            json_path = build_template_path / json_config_item["file"]

            if (
                json_config_item.get("skip_if_missing", False)
                and not json_path.exists()
            ):
                continue

            jq_arg = expand_template(json_config_item["jq"], value_template_env)

            LOGGER.info(f"Patching {json_path} with {jq_arg}")

            result = subprocess.run(
                [
                    "jq",
                    jq_arg,
                    json_path,
                ],
                capture_output=True,
            )

            if result.returncode != 0:
                LOGGER.error("jq stderr: %s\n%s", result.returncode, result.stderr)
                sys.exit(1)

            with open(json_path, "wb") as f:
                f.write(result.stdout)

A very basic manifest already goes from:

json_config:
  - file: "config/zcl/zcl_config.zap"
    jq: |
      (.endpointTypes[].clusters[].attributes[] | select(.name == "model identifier")).defaultValue = "DONGLE-E_R"

  - file: "config/zcl/zcl_config.zap"
    jq: |
      (.endpointTypes[].clusters[].attributes[] | select(.name == "manufacturer name")).defaultValue = "SONOFF"

  # remove OTA cluster
  - file: "config/zcl/zcl_config.zap"
    jq: |
      del(.endpointTypes[].clusters[] | select(.name == "Over the Air Bootloading"))

to:

json_config:
  - jq_func: 'zap_set_cluster_attribute("model identifier", "DONGLE-E_R", "all", 0)'
  - jq_func: 'zap_set_cluster_attribute("manufacturer name", "SONOFF", "all", 0)'
  - jq_func: 'zap_delete_cluster("Over the Air Bootloading", 1)'

More functions can be added as needed, but I think these two are a good start.

Note: this is definitely a rough draft, I'm sure there are a lot of optimizations to be done... It works though 😬

@puddly
Copy link
Collaborator Author

puddly commented Oct 30, 2024

My idea with using jq was to allow the json_config section to be completely generic. I'm not opposed to a bit of copy/paste for clarity, this is how it looks in my most recent iteration:

json_config:
  - file: "config/zcl/zcl_config.zap"
    jq: (.endpointTypes[].clusters[].attributes[] | select(.name == "model identifier")).defaultValue = $value
    args:
      value: "DONGLE-E_R"

  - file: "config/zcl/zcl_config.zap"
    jq: (.endpointTypes[].clusters[].attributes[] | select(.name == "manufacturer name")).defaultValue = $value
    args:
      value: "SONOFF"

  # Set software date code to day of build
  - file: "config/zcl/zcl_config.zap"
    jq: (.endpointTypes[].clusters[].attributes[] | select(.name == "date code")).defaultValue = $value
    args:
      value: template:{now:%Y%m%d%H%M%S}

  # Set software build ID to SDK version
  - file: "config/zcl/zcl_config.zap"
    jq: (.endpointTypes[].clusters[].attributes[] | select(.name == "sw build id")).defaultValue = $value
    args:
      value: template:{sdk_version}

  # remove OTA cluster
  - file: "config/zcl/zcl_config.zap"
    jq: del(.endpointTypes[].clusters[] | select(.name == "Over the Air Bootloading"))

  # remove Occupancy cluster
  - file: "config/zcl/zcl_config.zap"
    jq: del(.endpointTypes[].clusters[] | select(.name == "Occupancy Sensing"))

I'm not a fan of including any Python code in the YAML and evaling it, however. May as well rewrite the format to be ZAP specific and readable:

zap_config:
  file: "config/zcl/zcl_config.zap"
  cluster_defaults:
    "model identifier": "DONGLE-E_R"
    "manufacturer name": "SONOFF"
    "date code": template:{now:%Y%m%d%H%M%S}
  remove_clusters:
    - "Over the Air Bootloading"
    - "Occupancy Sensing"

@Nerivec
Copy link
Contributor

Nerivec commented Oct 30, 2024

The jq code is hugely typo-prone, I think abstracting the basics somehow would be better.
That zap_config seems indeed better than all the jq code.

There might be a better option with the actual zap (since it's in the container anyway) to do all this (https://github.com/project-chip/zap)? I haven't looked into what it can do/what's required/etc..

@darkxst
Copy link
Contributor

darkxst commented Oct 30, 2024

The jq code is hugely typo-prone, I think abstracting the basics somehow would be better.

I personally don't mind the jq code as a generic solution, and it is easy enough to test the queries externally if your worried about typos.

zap_config is cleaner and simpler, but what happens when you want to edit some other generic value, that hasn't been implemented in the builder.

There might be a better option with the actual zap (since it's in the container anyway)

The ZAP tool seems very much focussed around the GUI editor, there is support for templates for adding custom clusters and ensuring they are editible in the GUI. I dont think there is anyway to modify values from CLI commands, although documentation is pretty limited, and I didnt dig too deeply into the source code.

@Nerivec
Copy link
Contributor

Nerivec commented Oct 30, 2024

zap_config is cleaner and simpler, but what happens when you want to edit some other generic value, that hasn't been implemented in the builder.

Both would be kept, zap_config would just extend the json_config to a more legible format (since it would be the main use for the JSON support anyway). But you can still use json_config if you need something specific (though I suspect after ironing out the zap_config format, it would pretty much be set-and-done, it's not like there are that many operations to do on it).

zap_config:
  endpoint_types:
    - name: "Centralized"
      clusters:
        - name: "Basic"
          attribute_defaults:
              "model identifier": DONGLE-E_R
              "manufacturer name": SONOFF
              "date code": template:{now:%Y%m%d%H%M%S}
              "sw build id": template:{sdk_version}
          remove:
            - "Over the Air Bootloading"
    zap_config = manifest.get("zap_config", None)
    zap_json_config: list[dict[str, typing.Any]] = []

    if zap_config:
        for endpoint_type in zap_config.get("endpoint_types", []):
            for cluster in endpoint_type.get("clusters", []):
                for to_remove in cluster.get("remove", []):
                    zap_json_config += zap_delete_cluster(
                        to_remove, endpoint_type["name"]
                    )

                for attribute_name, default_value in cluster.get(
                    "attribute_defaults", {}
                ).items():
                    zap_json_config += zap_set_cluster_attribute(
                        attribute_name,
                        expand_template(default_value, value_template_env),
                        endpoint_type["name"],
                        cluster["name"],
                    )

    # JSON config
    for json_config in (
        DEFAULT_JSON_CONFIG + manifest.get("json_config", []) + zap_json_config
    ):

(with some adjusted versions of the utils from previous post, using .name instead of .id, not copied for brevity)

@darkxst
Copy link
Contributor

darkxst commented Oct 30, 2024

though I suspect after ironing out the zap_config format, it would pretty much be set-and-done, it's not like there are that many operations to do on it

In the context of a Zigbee router that is probably true, but lets say I now want to build a Thread Matter device, the requirements there are sure to differ from Zigbee use case.

@Nerivec
Copy link
Contributor

Nerivec commented Oct 30, 2024

Yes, but it's just a matter (/joke) of properly defining the format of the zap_config. The operations would differ, but remain in low count overall, wouldn't they (definitely no expert on Thread config)?
I think a couple of extra python functions are well-worth the much cleaner and less error-prone yaml.

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

Successfully merging this pull request may close these issues.

4 participants