Skip to content

Improve enum code generation#89

Merged
mikee47 merged 21 commits intodevelopfrom
feature/enum-ranges
Feb 5, 2026
Merged

Improve enum code generation#89
mikee47 merged 21 commits intodevelopfrom
feature/enum-ranges

Conversation

@mikee47
Copy link
Owner

@mikee47 mikee47 commented Feb 3, 2026

This PR presents an overhaul of code generation for enums to avoid duplicate code as described in #88.

toString functions are also generated so values can be used in printing expressions such as Serial << color << endl;.

For consistency, enumerated values are given range definitions, which is OK as the generated enum values are contiguous. This is handy for testing as we get typed random() function.

@mikee47 mikee47 merged commit f7a7106 into develop Feb 5, 2026
52 checks passed
@mikee47 mikee47 deleted the feature/enum-ranges branch February 5, 2026 17:22
@mikee47
Copy link
Owner Author

mikee47 commented Feb 5, 2026

@pljakobs Can you give the latest ConfigDB develop some exercise? When you're happy with it, let me know and I'll get a PR raised to update the main Sming branch.

@pljakobs
Copy link
Contributor

pljakobs commented Feb 5, 2026

initial test: compiles and seems to work as expected. (I did have to look up the syntax of the DSL, though :D)

    "telemetry": {
      "type": "object",
      "properties": {
        "statsEnabled": {
          "type":"string",
          "@default":"'ON' if SMING_RELEASE==0 else 'OFF'",
          "ctype":"telemetryStats",
          "enum":[ "ON", "OFF"]
        },
        "logEnabled": {
          "type":"string",
          "@default":"'ON' if SMING_RELEASE==0 else 'OFF'",
          "ctype":"telemetryLog",
          "enum":[ "ON", "OFF"]
      },
   }
}

I quite like this way of describing the default depending on the build type, I will have to look at other places, I might actually be able to get rid of conditionally compiled code through that.

One thing that I guess is not possible is a full array as a conditional, right? I have arrays for the valid pwm pins for the different SOCs and currently, the code must select the suitable one for the SOC it's running on - if I could have a [ 0, 1, 2, 3, 4 7 ] if SOC="esp32c3" else [ some other array] (actually I'd need more ... makes me wonder if the DSL needs a switch statement)

@pljakobs
Copy link
Contributor

pljakobs commented Feb 6, 2026

I'm currently testing the evaluated defaults and I am finding some weird behaviour, however, there are a number of layers here.
What would you think about the configdb code generator generating a report of generated default values? That would make it much easier to check that they are what one want them to be.
Otherwise, debugging those statements is either running the evaluator on the shell, hoping that one has injected the correct environment variables or flashing and looking at the result (which is rather simple in my case since I have an API endpont that gives me the full generated json, but it still requires a full flashinit/flash cycle)

having a table like
== calulated default values ==
.telemetry.stats.enabled = ON
.telemetry.logs.enabled = OFF

as part of the build output would be helpful

@pljakobs
Copy link
Contributor

pljakobs commented Feb 6, 2026

alternatively make configdb-show-defaults as an extra target that only outputs the defaults might be clearer

@pljakobs
Copy link
Contributor

pljakobs commented Feb 6, 2026

oh, I just noticed something that breaks the logic I had in mind:
configdb is not rebuilt unless the schema files have changed.
That sort of defeats the original purpose: we now need to rebuild configdb on every build, as the change now can be in the environment.
That explains my observations here: I can change the build type (SMING_RELEASE) as much as I want, that will not change the default if it's done in the same builddir.
I believe that should not be an issue on matrix builds in github actions as they get a fresh builddir every time, but for local builds, this is very unintuitive.

@pljakobs
Copy link
Contributor

pljakobs commented Feb 6, 2026

or maybe, to not compile unconditionally at all times, we could create a file holding the last values of all parameters used in the evaluation and evaluate if one of those has changed? Sounds a bit complex for something that doesn't take a huge amount of time to build, though.

@pljakobs
Copy link
Contributor

pljakobs commented Feb 6, 2026

if the evaluator script would generate a file with the values of all variables it has used in the build pass along with their values, would that be something the makefile could easily compare to the current environment to decide if configdb needs to be rebuilt?

@mikee47
Copy link
Owner Author

mikee47 commented Feb 6, 2026

Evaluator patch to support lists:

diff --git a/Tools/Python/evaluator.py b/Tools/Python/evaluator.py
index 321ffa4a..70a29e33 100755
--- a/Tools/Python/evaluator.py
+++ b/Tools/Python/evaluator.py
@@ -139,6 +139,9 @@ class Evaluator:
         if isinstance(node, ast.JoinedStr):
             return ''.join(str(self._eval(v)) for v in node.values)
 
+        if isinstance(node, ast.List):
+            return [self._eval(v) for v in node.elts]
+
         raise TypeError(f"Unsupported syntax: {type(node).__name__}")
 
     def run(self, expr):

@mikee47
Copy link
Owner Author

mikee47 commented Feb 6, 2026

I'm currently testing the evaluated defaults and I am finding some weird behaviour, however, there are a number of layers here. What would you think about the configdb code generator generating a report of generated default values? That would make it much easier to check that they are what one want them to be. Otherwise, debugging those statements is either running the evaluator on the shell, hoping that one has injected the correct environment variables or flashing and looking at the result (which is rather simple in my case since I have an API endpont that gives me the full generated json, but it still requires a full flashinit/flash cycle)

having a table like
== calulated default values ==
.telemetry.stats.enabled = ON
.telemetry.logs.enabled = OFF

as part of the build output would be helpful

alternatively make configdb-show-defaults as an extra target that only outputs the defaults might be clearer

That should be pretty straightforward to do.
For now, take a look in e.g. out/Esp8266/Debug/ConfigDB/schema. Those .json files are post-eval but contain the full schema, not just the evaluated bits.

@mikee47
Copy link
Owner Author

mikee47 commented Feb 6, 2026

oh, I just noticed something that breaks the logic I had in mind: configdb is not rebuilt unless the schema files have changed. That sort of defeats the original purpose: we now need to rebuild configdb on every build, as the change now can be in the environment. That explains my observations here: I can change the build type (SMING_RELEASE) as much as I want, that will not change the default if it's done in the same builddir. I believe that should not be an issue on matrix builds in github actions as they get a fresh builddir every time, but for local builds, this is very unintuitive.

PR #82 changed the output directory for generated code from out/ConfigDB to e.g. out/Esp8266/debug/ConfigDB.
So in fact changing the build type via SMING_RELEASE will result in a rebuild.

@mikee47
Copy link
Owner Author

mikee47 commented Feb 6, 2026

or maybe, to not compile unconditionally at all times, we could create a file holding the last values of all parameters used in the evaluation and evaluate if one of those has changed? Sounds a bit complex for something that doesn't take a huge amount of time to build, though.
if the evaluator script would generate a file with the values of all variables it has used in the build pass along with their values, would that be something the makefile could easily compare to the current environment to decide if configdb needs to be rebuilt?

This is a similar problem to library Components https://sming.readthedocs.io/en/latest/_inc/Sming/building.html#library-variants.
Sming handles this using directory names which are a hash of the dependent build variables.

Perhaps a similar scheme could be adopted for ConfigDB?

@pljakobs
Copy link
Contributor

pljakobs commented Feb 6, 2026

PR #82 changed the output directory for
generated code from out/ConfigDB to e.g. out/Esp8266/debug/ConfigDB.
So in fact changing the build type via SMING_RELEASE will result in a rebuild.

that is not consistent with what I see, that was my challenge, just changing the release flag did not result in updated defaults.

@pljakobs
Copy link
Contributor

pljakobs commented Feb 6, 2026

This is a similar problem to library Components https://sming.readthedocs.io/en/latest/_inc/Sming/building.html#library-variants.
Sming handles this using directory names which are a hash of the dependent build variables.

Perhaps a similar scheme could be adopted for ConfigDB?

I have a version of avaluator.py that creates an env_var file of format

VAR = <value>

and appends to that with new vars in every new run.
It expects to be cleared at the beginning of a configdb build, as the evaluator itself does not have the full context of the schema.
At the end of the build, it should list the complete state of the used environment and thus might be a basis to decide if we need to re-compile?

@mikee47
Copy link
Owner Author

mikee47 commented Feb 6, 2026

PR #82 changed the output directory for
generated code from out/ConfigDB to e.g. out/Esp8266/debug/ConfigDB.
So in fact changing the build type via SMING_RELEASE will result in a rebuild.

that is not consistent with what I see, that was my challenge, just changing the release flag did not result in updated defaults.

You need to change your logic to

"@default":"'OFF' if SMING_RELEASE==1 else 'ON'"

SMING_RELEASE will only ever have the value "1" or "". See Sming/build.mk line 71.

https://sming.readthedocs.io/en/latest/_inc/Sming/index.html#release-builds

(That means "@default":"'OFF' if SMING_RELEASE else 'ON'" is also fine.)

@pljakobs
Copy link
Contributor

pljakobs commented Feb 6, 2026

Ah, that explains what I saw.

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.

2 participants