-
Notifications
You must be signed in to change notification settings - Fork 4
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
Replicating complexity in builder.py in ibek yaml #167
Comments
Don't you have to explicitly add the 8 lines to add |
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. |
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 |
I remember the conversation but not the use of enums... I was thinking more:
|
yep - this is why IRL conversations are a bit rubbish. |
I'm pretty sure we have this covered in 3.0.0b6.
Leaving open to look at cleaning up the |
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:
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?
The text was updated successfully, but these errors were encountered: