epics-containers / ibek

IOC Builder for EPICS and Kubernetes
https://epics-containers.github.io/ibek
Apache License 2.0
11 stars 5 forks source link

Replicating complexity in builder.py in ibek yaml #167

Open gilesknap opened 8 months ago

gilesknap commented 8 months ago

There are some cases where the builder.py is doing things that we probably should replicate in ibek support YAML.

e.g. in the asyn module we have a tidy way of adding configuration items to the asyn object as follows:

    def Initialise(self):
        print('%sConfigure("%s", "%s", %d, %d, %d)' % (
            self.DbdFileList[0], self.asyn_name, self.port_name, self.priority,
            self.noAutoConnect, self.noProcessEos))
        for key, value in self.options.items():
            print('asynSetOption("%s", 0, "%s", "%s")' % (
                self.asyn_name, key, value))
        if self.input_eos is not None:
            print('asynOctetSetInputEos("%s", 0, %s)' % (
                self.asyn_name, quote_c_string(self.input_eos)))
        if self.output_eos is not None:
            print('asynOctetSetOutputEos("%s", 0, %s)' % (
                self.asyn_name, quote_c_string(self.output_eos)))

In particular the options dictionary is really useful here. Also inheritance allows for this pattern to be reused in a few Asyn classes.

At present I have this in YAML which is adequate for the simple streamdevice I have made so far but is pretty limited and the approach does not scale.

https://github.com/epics-containers/ibek-support/blob/14384f341a8000d31c3013ec3b5bd618aa0d17f2/asyn/asyn.ibek.support.yaml#L99C1-L112C93

@coretl @GDYendell do you think we should support dictionary arguments for this purpose?

GDYendell commented 8 months ago

Don't you have to explicitly add the 8 lines to add disconnectOnReadTimeout as an arg in the support yaml anyway? Or do you want to be able to provide any arbitrary key-value to be called as asynSetOption in the ioc yaml, without having to add it to the support yaml?

gilesknap commented 8 months ago

I think the key value pair is the best way to achieve this without lots of lines of YAML. but then there is no build time checking.

I think I'm OK with needing an arg for each option but I don't want the pre_init to get really long and unreadable because of too many jinga ifs.

gilesknap commented 8 months ago

I discussed this with Tom, and he suggested:

Have a argument for every possible option and then make the startup jinja look like this

{% for option in [option1, option2, option3] %}
asynSetOption( {{ name }}, 0, {{ option.name }}, {{ option.value }} )
{% endfor %}

Seems tidy enough to me

coretl commented 8 months ago

I remember the conversation but not the use of enums...

I was thinking more:

{% for attr in ["baud", "parity",... ] %}
asynSetOption( {{ name }}, 0, {{ attr }}, {{ locals()[attr] }} )
{% endfor %}
gilesknap commented 8 months ago

yep - this is why IRL conversations are a bit rubbish.

gilesknap commented 3 months ago

I'm pretty sure we have this covered in 3.0.0b6.

Leaving open to look at cleaning up the asynSetOption feature in Asyn module