[RESEND, RFC 1/8] tools: config: yaml: Add board config class to generate config binaries
Alper Nebi Yasak
alpernebiyasak at gmail.com
Mon Apr 18 21:55:56 CEST 2022
On 06/04/2022 15:29, Neha Malcom Francis wrote:
> For validating config files and generating binary config artifacts, here
> board specific config class is added.
>
> Add function cfgBinaryGen() in tibcfg_gen.py. It uses TIBoardConfig
> class to load given schema and config files in YAML, validate them and
> generate binaries.
The subject lines (of other patches as well) sound too generic when most
of them are TI specific, I'd expect at least a 'ti:' tag except where
you already include more specific terms like a board name.
(This one could be "tools: ti: Add ..." for example).
>
> Signed-off-by: Tarun Sahu <t-sahu at ti.com>
> [n-francis at ti.com: prepared patch for upstreaming]
> Signed-off-by: Neha Malcom Francis <n-francis at ti.com>
> ---
> test/py/requirements.txt | 1 +
> tools/tibcfg_gen.py | 116 +++++++++++++++++++++++++++++++++++++++
> 2 files changed, 117 insertions(+)
> create mode 100644 tools/tibcfg_gen.py
>
> diff --git a/test/py/requirements.txt b/test/py/requirements.txt
> index 33c5c0bbc4..a91ba64563 100644
> --- a/test/py/requirements.txt
> +++ b/test/py/requirements.txt
> @@ -4,6 +4,7 @@ coverage==4.5.4
> extras==1.0.0
> fixtures==3.0.0
> importlib-metadata==0.23
> +jsonschema==4.0.0
> linecache2==1.0.0
> more-itertools==7.2.0
> packaging==19.2
> diff --git a/tools/tibcfg_gen.py b/tools/tibcfg_gen.py
> new file mode 100644
> index 0000000000..7635596906
> --- /dev/null
> +++ b/tools/tibcfg_gen.py
> @@ -0,0 +1,116 @@
> +# SPDX-License-Identifier: GPL-2.0+
> +# Copyright (C) 2022 Texas Instruments Incorporated - https://www.ti.com/
> +#
> +# TI Board Configuration Class for Schema Validation and Binary Generation
> +#
> +
> +from jsonschema import validate
> +
> +import yaml
> +import os
> +import sys
Standard library imports should appear before third-party libraries,
with an empty line between them.
> +
> +
> +class TIBoardConfig:
> + file_yaml = {}
> + schema_yaml = {}
> + data_rules = {}
These belong in __init__ as they are per-instance attributes.
> +
> + def __init__(self):
> + pass
> +
> + def Load(self, file, schema, data_rules=""):
You can rename this to be the __init__ function.
> + with open(file, 'r') as f:
> + self.file_yaml = yaml.safe_load(f)
> + with open(schema, 'r') as sch:
> + self.schema_yaml = yaml.safe_load(sch)
> + self.data_rules = data_rules
> +
> + def CheckValidity(self):
> + try:
> + validate(self.file_yaml, self.schema_yaml)
> + return True
> + except Exception as e:
> + print(e)
> + return False
You can also do this validation immediately after loading the yaml files
in the __init__(), and then safely assume any created object is valid.
> +
> + def __ConvertToByteChunk(self, val, data_type):
Methods should be in snake_case. Also consider using a single underscore
as the prefix, double underscore does some special name mangling.
> + br = []
> + size = 0
> + if(data_type == "#/definitions/u8"):
> + size = 1
> + elif(data_type == "#/definitions/u16"):
> + size = 2
> + elif(data_type == "#/definitions/u32"):
> + size = 4
> + else:
> + return -1
I think this case should raise an error of some kind.
> + if(type(val) == int):
> + while(val != 0):
In general, don't use parentheses with if, while etc.
> + br = br + [(val & 0xFF)]
> + val = val >> 8
> + while(len(br) < size):
> + br = br + [0]
> + return br
This all looks like val.to_bytes(size, 'little'), but as a list instead
of bytes. If you want to get fancy, have a look at the struct module.
(For example, struct.pack('<L', val) )
> +
> + def __CompileYaml(self, schema_yaml, file_yaml):
> + br = []
Consider using a bytearray instead of a list-of-ints here.
> + for key in file_yaml.keys():
I think things would be more readable if you extracted
node = file_yaml[key]
node_schema = schema_yaml['properties'][key]
node_type = node_schema.get('type')
as variables here and used those in the following code.
> + if not 'type' in schema_yaml['properties'][key]:
> + br = br + \
br += ... would be nicer for all of these.
> + self.__ConvertToByteChunk(
> + file_yaml[key], schema_yaml['properties'][key]["$ref"])
> + elif schema_yaml['properties'][key]['type'] == 'object':
> + br = br + \
> + self.__CompileYaml(
> + schema_yaml['properties'][key], file_yaml[key])
> + elif schema_yaml['properties'][key]['type'] == 'array':
> + for item in file_yaml[key]:
> + if not isinstance(item, dict):
> + br = br + \
> + self.__ConvertToByteChunk(
> + item, schema_yaml['properties'][key]['items']["$ref"])
> + else:
> + br = br + \
> + self.__CompileYaml(
> + schema_yaml['properties'][key]['items'], item)
> + return br
> +
> + def GenerateBinaries(self, out_path=""):
> + if not os.path.isdir(out_path):
> + os.mkdir(out_path)
> + if(self.CheckValidity()):
> + for key in self.file_yaml.keys():
> + br = []
You don't need this assignment, it's overwritten in the next one anyway.
> + br = self.__CompileYaml(
> + self.schema_yaml['properties'][key], self.file_yaml[key])
> + with open(out_path + "/" + key + ".bin", 'wb') as cfg:
Construct file paths with os.path.join() here and below.
> + cfg.write(bytearray(br))
> + else:
> + raise ValueError("Config YAML Validation failed!")
> +
> + def DeleteBinaries(self, out_path=""):
> + if os.path.isdir(out_path):
> + for key in self.file_yaml.keys():
> + if os.path.isfile(out_path + "/" + key + ".bin"):
> + os.remove(out_path + "/" + key + ".bin")
> +
> +
> +def cfgBinaryGen():
> + """Generate config binaries from YAML config file and YAML schema
> + Arguments:
> + - config_yaml: board config file in YAML
> + - schema_yaml: schema file in YAML to validate config_yaml against
> + Pass the arguments along with the filename in the Makefile.
> + """
> + tibcfg = TIBoardConfig()
> + config_yaml = sys.argv[1]
> + schema_yaml = sys.argv[2]
> + try:
> + tibcfg.Load(config_yaml, schema_yaml)
> + except:
> + raise ValueError("Could not find config files!")
> + tibcfg.GenerateBinaries(os.environ['O'])
I think it'd be better to pass the directory as an -o / --output-dir
argument instead of reading it from environment. You can use argparse to
parse the command line arguments.
> +
> +
> +cfgBinaryGen()
This should be guarded by if __name__ == '__main__'.
More information about the U-Boot
mailing list