EPICS Controls Argonne National Laboratory

Experimental Physics and
Industrial Control System

2002  2003  2004  2005  2006  2007  2008  2009  2010  2011  <20122013  2014  2015  2016  2017  2018  2019  2020  2021  2022  2023  2024  Index 2002  2003  2004  2005  2006  2007  2008  2009  2010  2011  <20122013  2014  2015  2016  2017  2018  2019  2020  2021  2022  2023  2024 
<== Date ==> <== Thread ==>

Subject: Re: [Merge] lp:~jlmuir/epics-base/iocsh-if-flow-control-3.14 into lp:epics-base
From: Andrew Johnson <[email protected]>
To: [email protected]
Date: Tue, 10 Jan 2012 19:45:26 -0000
Hi Lewis,

You wrote:
> 7. $ bzr push lp:~jlmuir/epics-base/iocsh-if-flow-control-3.14
> 8. Click "Propose for merging" link in the view of my pushed branch
> 
> So I guess my mistake was in step 7 where lp:~jlmuir/epics-base is actually
> pointing to the 3.15 branch?  To have done this correctly, I think I would
> have needed to make step 7 the following:
> 
> 7. $ bzr push lp:~jlmuir/epics-base/3.14/iocsh-if-flow-control-3.14
> 
> Is that correct?

No, I think that path would get rejected by Bazaar.  You select the branch you're proposing to merge into on the Launchpad "Propose Branch for merging" page (the first form field, labelled Target branch).  However we are only accepting new features into the 3.15 branch now, so Launchpad's default was correct; if you made a mistake anywhere it was in checking out and working from the 3.14 branch.

<snip>
> My feeling is that the above is a kludge.  Why try to hack a .cmd file to
> do what I want when I can add support for an if-statement and have it do
> what I want cleanly?

That's the Law of conservation of complexity in operation; the code you're responsible for gets slightly simpler but an important part of Base gets more complex.  I would prefer to suggest ways to reduce your complexity without having to increase mine.

> The '$(PILATUS_ENABLED=#) dbpf("x7PIL1:det1:TriggerMode","Ext. Trigger")'
> style is weak in that it makes lines even longer, is less readable, and can
> only handle Boolean cases.

It's easy to shorten the variable names.  In this example lines starting with $(T1) are only run when TEST1_ENABLED is YES, and $(T2) lines run only when TEST2_ENABLED is YES:

===

epicsEnvSet RUN_YES ""
epicsEnvSet RUN_NO "#"
epicsEnvSet T1 "$(RUN_$(TEST1_ENABLED))"
epicsEnvSet T2 "$(RUN_$(TEST2_ENABLED))"

## Load record instances
$(T1) dbLoadRecords "db/test1.db"
$(T2) dbLoadRecords "db/test2.db"

===

Here are a couple of your later examples using that technique.  Note that this approach can also be extended in various ways.

===

epicsEnvSet RUN_YES ""
epicsEnvSet RUN_NO "#"
epicsEnvSet PE "$(RUN_$(PILATUS_ENABLED))"

#
# Configure asyn port for communicating with PILATUS camserver
#
${PE} drvAsynIPPortConfigure("camserver","10.211.55.17:41234")
${PE} asynOctetSetInputEos("camserver",0,"\030")
${PE} asynOctetSetOutputEos("camserver",0,"\n")

#
# pilatusDetectorConfig(const char *portName, const char *camserverPort,
#     int maxSizeX, int maxSizeY, int maxBuffers, size_t maxMemory,
#     int priority, int stackSize)
#   portName       driver asyn port name
#   camserverPort  server asyn port name
#   maxSizeX       size of PILATUS detector in X direction
#   maxSizeY       size of PILATUS detector in Y direction
#   maxBuffers     maximum number of buffers for NDArrayPool
#   maxMemory      maximum amount of memory in bytes for NDArrayPool
#   priority       thread priority for asyn port driver thread if
#                  ASYN_CANBLOCK is set in asynFlags
#   stackSize      stack size for asyn port driver thread if ASYN_CANBLOCK is
#                  set in asynFlags
#
${PE} pilatusDetectorConfig("PIL1", "camserver", 2463, 2527, 50, 200000000)
${PE} dbLoadRecords("$(AREA_DETECTOR)/ADApp/Db/ADBase.template", "P=x7PIL1:,R=det1:,PORT=PIL1,ADDR=0,TIMEOUT=1")
${PE} dbLoadRecords("$(AREA_DETECTOR)/ADApp/Db/NDFile.template", "P=x7PIL1:,R=det1:,PORT=PIL1,ADDR=0,TIMEOUT=1")
${PE} dbLoadRecords("$(AREA_DETECTOR)/ADApp/Db/pilatus.template","P=x7PIL1:,R=det1:,PORT=PIL1,ADDR=0,TIMEOUT=1,CAMSERVER_PORT=camserver")

===

> And as I said, the "${PILATUS_ENABLED=#}" style can only handle Boolean
> cases.  Here's an example of a non-Boolean environment variable use:

The following is no more complex than your version, and if you were to add more detectors I think it would scale better than a series of if statements:

===

epicsEnvSet ADPV_ADSC "x7ADSC1:det1:"
epicsEnvSet ADPV_PILATUS "x7PIL1:det1:"
epicsEnvSet EXPHI_ADSC No
epicsEnvSet EXPHI_PILATUS Yes

# Attach imcais to areaDetector driver for simulated ADSC Q210
dbpf("ioc23:Is:AdDrvBasePvName.VAL","$(ADPV_$(DETECTOR))")
# Enable/Disable exphi interface
dbpf("ioc23:Is:UseExphi.VAL","$(EXPHI_$(DETECTOR))")

===

> With an if-statement, I can also do consistency checking on my
> configuration environment variables.  For example:

You can do similar checks with all kinds of logic in them without having to use an if statement, using variable names with multiple expanded-out parts to them for example.  I don't want to spend time redoing all of your examples though, I'm sure you can work them out from the above ideas, but let me know if you come across anything particularly tricky.

> Obviously, the other things supported in the if-statement feature have not
> been talked about.  This is because I don't actually have a real world need
> for them right now.  But I'll just note that performing a Boolean "or" is
> not possible with the "${PILATUS_ENABLED=#}" style.  And the integer
> comparison operators don't have a "${PILATUS_ENABLED=#}" style solution
> either.

See above; OR, AND and SELECT operations are all possible without having to introduce another expression evaluation engine into Base.

> This is where I don't know what other people are wanting.  All I know is
> that, even though iocsh can only do a very limited number of things, it
> works OK for me, *except* for this conditional evaluation problem (and line
> continuation, but we're not talking about that here :-)).  It's easy to
> make the claim that if conditionals are added, then people will want
> whatever other feature.  That could happen, or it might not happen.  Maybe
> conditionals will be enough to handle most needs like it would be for me. 
> And you can obviously say no to future feature requests; you don't have to
> be "fair."  So if this if-statement feature is reasonable to you, you can
> accept it but reject other requests.

I believe that iocsh and macLib together can do more than you think they can, without the resulting script becoming hard to understand.  I am reluctant to add complexity to the code in iocsh.cpp since I don't believe it's necessary to solve your problems.

If the community sees a need for a better scripting language I'll be happy to look at embedding Lua as an alternative shell — it's not the Lua language or syntax that I'm focussing on though (I've never written a line of Lua in my life BTW), but the fact that the Lua implementation is widely recognized as small, well-designed and easily embedable, and we won't have to maintain it ourselves.

- Andrew

-- 
https://code.launchpad.net/~jlmuir/epics-base/iocsh-if-flow-control-3.14/+merge/87972
Your team EPICS Core Developers is requested to review the proposed merge of lp:~jlmuir/epics-base/iocsh-if-flow-control-3.14 into lp:epics-base.


References:
[Merge] lp:~jlmuir/epics-base/iocsh-if-flow-control-3.14 into lp:epics-base J. Lewis Muir

Navigate by Date:
Prev: Re: [Merge] lp:~jlmuir/epics-base/iocsh-if-flow-control-3.14 into lp:epics-base J. Lewis Muir
Next: [Merge] lp:~jlmuir/epics-base/iocsh-comment-fix-3.14 into lp:epics-base/3.14 J. Lewis Muir
Index: 2002  2003  2004  2005  2006  2007  2008  2009  2010  2011  <20122013  2014  2015  2016  2017  2018  2019  2020  2021  2022  2023  2024 
Navigate by Thread:
Prev: Re: [Merge] lp:~jlmuir/epics-base/iocsh-if-flow-control-3.14 into lp:epics-base J. Lewis Muir
Next: [Merge] lp:~jlmuir/epics-base/iocsh-if-flow-control-3.14 into lp:epics-base Andrew Johnson
Index: 2002  2003  2004  2005  2006  2007  2008  2009  2010  2011  <20122013  2014  2015  2016  2017  2018  2019  2020  2021  2022  2023  2024 
ANJ, 26 Nov 2012 Valid HTML 4.01! · Home · News · About · Base · Modules · Extensions · Distributions · Download ·
· Search · EPICS V4 · IRMIS · Talk · Bugs · Documents · Links · Licensing ·