search

pcdshub / engineering_tools

A repository of scripts, configuration useful for the PCDS team
Other
4 stars 27 forks source link

Changes to camViewer #94

Closed spenc333 closed 2 years ago

spenc333 commented 2 years ago

Description

Changes to camviewer script involved making fully worded enable and disable options as well fixing the -H option that now allows for a camera to be accessed from any hutch. Adding the imgr script to eng_tools instead of its original directory allows for control of an ioc given it's name and hutch . Also needed this script to be called in camviewer

Motivation and Context

Fixes #76

How Has This Been Tested?

Tests involved sourcing the script from my epics-dev directory using various camera names and hutches as well as the other options. Since imgr is in development and needed this command within camviewer, it also had to be sourced from epics-dev directory within the camviewer script.

Where Has This Been Documented?

Both scripts have updated usage functions

ZLLentz commented 2 years ago

One more suggestion for any time you're doing a bash script: make sure you run the result through shellcheck, it can often help catch issues. There's a shellcheck utility in pcds_conda and also a website https://www.shellcheck.net/

silkenelson commented 2 years ago

It feels wrong to me to add the imgr script to engineering tools as long as it also lives in the iocmanager package - with exactly the same name.

spenc333 commented 2 years ago

It feels wrong to me to add the imgr script to engineering tools as long as it also lives in the iocmanager package - with exactly the same name.

I agree.

ZryletTC commented 2 years ago

@silkenelson This is pretty much how iocmanager is done as well, do you have a better alternative? At the moment, there is no default way to use imgr than by specifying the full path to the executable. Adding it to engineering_tools adds it to everyone's PATH without extending the PATH variable. I brought up the question of how to handle this with you and Mike over Slack back in June. He agreed that a wrapper script in engineering_tools was probably best but I never heard your opinion. If there's a better way, I'm all ears.

silkenelson commented 2 years ago

If I was part of that discussion, I have forgotten - I am unable to retain slack discussions for much longer than a week. Yes indeed: the answer that removing this script from iocmanager was part of this request (or at least where this was going) answers my question.

ZryletTC commented 2 years ago

Once #101 is merged, the imgr addition should be removed from this script, then I'll rereview this PR and we can hopefully merge.

ZLLentz commented 2 years ago

I'm excited to use this one, I semi-frequently find myself wishing I had the hutch selector here