-
Notifications
You must be signed in to change notification settings - Fork 1
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
Refactor iocsh template to become reusable for deploy #17
Open
henriquesimoes
wants to merge
19
commits into
lnls-production
Choose a base branch
from
template
base: lnls-production
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+276
−255
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
These settings have been around but the ones that make it effective were commented out. Clean up everything related to autosave until we configure it properly as an optional standalone iocsh template.
This file should contain only commands actively in use. For history of the changes, one should rely the one version control tooling instead.
Template files are intended to be loaded by other iocsh scripts, and not to be run directly. Drop the shebang to avoid misguiding users, and also their execution bit accordingly.
These scripts are intended to be run directly, and load other scripts. As such, they also require the execution bit set. Fix that.
Since EPICS 3.16.1 [1], EPICS_CA_MAX_ARRAY_BYTES is optional. Drop it from all main iocsh scripts to simplify IOC configuration. [1] https://github.com/epics-base/epics-base/blob/9f8a8b9c1fc96dcbb18a35b627530428f74aaa62/documentation/RELEASE_NOTES.md#epics_ca_max_array_bytes-is-optional
These calibration files often differ by name and location in the filesystem for each deploy. As such, it doesn't make sense to include it hard-coded in the template file. Make it a proper environment variable to be defined by the iocsh script that uses the template. Calibration path defined in each model's main script is merely illustrative, and should be changed by users in their own instance of the main iocsh script. To give an educated example, use a FHS compliant path.
This plugin set has been imported back in 09e378f (Added commons plugins, and corrected corruption in array data. was deleting the buffer that was continously being used when the reference count reaches 0, 2020-12-15), and its description was never updated according to its usage here. We won't use it as an example, but instead as the actual set of plugins to be used at runtime, since we load it in the pimega-template.cmd. Also, we are not loading autosave requirements here. Remove those references from the description.
There is no other set of plugin settings that is not "common". Drop this over-specification from the file name.
While NDPluginStdArrays is closely related to the device itself, it will hardly be instantiated differently (in number and settings) from one detector to another. Thus, place all plugins in same template, making it easier to track where these settings are, besides turning the base template more self-contained and reusable.
Because some settings must be done after iocInit(), and they should be shared among all models, extract these steps into a standalone script to be loaded by each main script afterwards. This is a step towards moving the iocInit() back to the main script, giving users the ability to control what should be done right before and after initialization.
Some commands must be executed before iocInit, but also after the driver port has been already created, for example, instantiating new plugins. Having iocInit() in the very same template that creates the driver makes it impossible to add any such commands. Give back the flexibility of customizing the main script by moving the iocInit to the main script.
Plugins have specific definitions that can be provided when instantiating them or loading their templates. These definitions are hardly changed between deploys. As such, define them as the default value to simplify main start-up scripts. We document them in a dedicated section of variables, so that is clear which ones are required or optional.
Although listed under the "Plugins" section, this variable is also used by the pimegaDetector command to instantiate the detector driver. Move it to a more general place to avoid misguiding readers. Having this close to the detector model is interesting because maximum image size will generally be in function of the model.
Loading the default plugin collection is something that is up to the user choose to do. Otherwise, if one won't want to load one type of plugin, they will need to stop using the whole device template. Load that in the main script for each example template instead. Also move loading the device template upwards, to make clearer that some variables are only plugin specific and belong to their own logical block.
There are multiple environment variables that must be set in order to use this template. In order to avoid having users search the entire file looking for their usage and meaning, move all pimegaDetector parameters to the top of the file, and make it more focused on the environment variables to be defined. Notice that variables already defined in envPaths are not documented, since they should always be loaded and not defined by the users.
The template itself is the source of truth for the meaning of the variables, and it properly documents it. Drop these redudant comments from the file what will be the base for each deploy, and replicated for each model here.
These examples have been replicated based on the 135D model, but their prefix were not even updated accordingly. Fix that. While here, also remove the duplicate reference to "PIMEGA" in this prefix. Instead, use "DET" which can refer both to "detector" in a general term and, for LNLS, "Detectors Group".
We don't need a long name to reference each model. Let's make it straightforward instead. We keep the `st-` prefix to keep the reference they are the classical "st.cmd" scripts and to make them sorted together even when new models come in.
To make it easier for a human to parse the definitions, group them visually into four blocks: - basic settings - IP addresses - connection ports - miscellaneous variables These blocks seem good by now, but they surely can be redefined as new variables are introduced or existing ones removed.
4e5002e
to
7dc465c
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We first introduced a main iocsh template for PIMEGA in c5fe602 so that most configuration is not replicated across multiple model start-up files in the IOC repository. However, such template is hard to use in deploy due to its lack of flexibility, e.g. plugins addition, because it both creates the detector port and calls
iocInit
by itself. This patch series intends to make them more easily reusable across deploy as well, by restructuring the main template and better documenting the templates themselves.When working on this, I also cleaned up some things in preparation to this main change. They all come as the first patches in the series. The last patches also change some minor styling and organizational nits.
As always, I recommend reviewing this patch-wise in their commit order. ;)