RE: A question regarding sequencer compatibility

[Mark Rivers <rivers@cars.uchicago.edu>]

I am fine with all of your recommended changes.

I just search the APS "synApps" tree and all of our application files, and the change you propose for seq_pv(Put,Get) would only require edits to 4 files.

Mark


-----Original Message-----
From: tech-talk-bounces@aps.anl.gov [mailto:tech-talk-bounces@aps.anl.gov] On Behalf Of Benjamin Franksen
Sent: Tuesday, October 01, 2013 9:48 AM
To: tech-talk@aps.anl.gov
Subject: Re: A question regarding sequencer compatibility

someone replied:
> I quite like the idea of leaving the current functions as is and
> adding new functions such as your example of pvGetArray.

and someone else:
> I vote for the first option. pvGet/Put on arrays sounds very natural
> for me.

Hm, two answers, two opinions. What did I expect from such a poll? Well 
at least I'm not being spammed with hate-mail... ;-)

Let me first repeat that I am talking about version 2.2; in version 2.1 
everything will remain as is (bugfixes notwithstanding).

I think "leaving the current functions as is" is not a good option. It 
would mean I have to indefinitely support an inconsistent and 
unintuitive interface. My time for working on the sequencer is limited 
and if I get bugged down with maintaining strict compatibility forever, 
I'll not be able to make progress towards new features.

However, silently changing the behaviour at runtime would be even worse! 
You'd have to manually scan all your SNL sources before switching to 
some 2.1 version, checking the variable's PV-multiplicity for each call, 
then decide whether to change the code or not. Forcing users to undergo 
such a boring and error prone process for an upgrade would be evil.

My current plan is therefore to give a compile time error if pv 
functions are called with multi-PV arrays, as they do not properly 
support them anyway. This means the compiler helps you by pointing to 
the location in your source code where a change is needed. Either you 
append "[0]" to the variable if that is actually the intention. Or it's 
not, and as a result a few errors will be uncovered and wouldn't that be 
nice?[*]

What to do with pvPutComplete? It is the only function that properly 
supports multi-PV arrays. I think I will remove this feature so that it 
behaves in manner consistent with pvGetComplete, and then add a new 
function that implements the more general behaviour. Again, this means 
you might have to slightly change your program but the compiler will 
help you in figuring out what to do.

BTW, on the subject of how to name the new functions (no bike-shedding 
ensued, are you all asleep?) I am currently in favour of using pural 
form i.e. pvGets, pvPuts, pvGetsComplete, etc. I don't like pvGetArray 
since (1) the normal pvGet works perfectly fine with arrays (only not 
with multi-PV arrays) and (2) the new function will work with single-PV 
variables, too.

 * * *

And now to another issue: Currently, the timeout for synchronous pvGet 
and pvPut is hard-coded to 10 seconds. Adding an extra (optional) 
argument to specify a different timout has been on my wish-list for a 
long time. This is now implemented (in the 2.2 branch) but again an 
issue of compatibility comes up, though in this case it does not concern 
SNL code proper (adding an extra DEFAULT_TIMEOUT argument can be done 
automatically by the compiler). However, escaped C code that calls 
seq_Pv{Put,Get} will have to be changed to add this argument. Note that 
here, too, the compiler (the C compiler in this case) poinpoints the 
location of the problem for you.

I am in favour of making this change in version 2.2, but if it turns out 
to hurt people too much I am willing to restrict the change to the new 
pvGets and pvPuts.

Cheers
Ben

[*] The difference between using pvGetComplete(x[0]) and, for instance, 
pvGetComplete(x[0]) && pvGetComplete(x[1]) in a when-clause is hard to 
detect with tests, since completion is normally very fast, even if the 
PVs reside on another IOC. There is a high probability that by the time 
the action block is entered and x[1] is accessed, its callback has 
indeed arrived, even if the citerion was to check this only for x[0]. 
Which means the program will work most of the time. In my automated 
tests I usually need 10000s of runs to get a single failure...

Am Sonntag, 29. September 2013, 03:31:02 schrieb Benjamin Franksen:
> Hi Everyone
> 
> until now I have tried hard to avoid breaking existing SNL programs
> when developing new features or cleaning up the code base. I cannot
> claim that I succeeded completely, but I got the impression that the
> transition from 2.0 to 2.1 went reasonably well for most people.
> 
> I am planning to make some changes for version 2.2 that can break
> existing programs if they rely on certain mis-features that I
> inherited from earlier versions.
> 
> One of these is the behaviour of pvPut and pvGet with multi-PV arrays.
> Concretely, assume a declaration
> 
> 	int x[2];
> 	assign x to { "pv1", "pv1" };
> 
> and an action statement
> 
> 	pvGet(x);
> 
> Currently this is equivalent to
> 
> 	pvGet(x[0]);
> 
> I find this quite un-intuitive and arbitrary. My current plan is to
> change the behaviour so that if an array has been assigned to multiple
> PVs, the pv functions should act on *all* elements, instead of only
> the first.
> 
> You might think at first that this is unnecessary, why not implement
> the behaviour with a simple loop:
> 
> 	for(i=0; i<2; i++)
> 		pvGet(x[i]);
> 
> But this would have far greater latency than necessary, since it
> issues one request, waits for the response, then issues the next
> request, waits again, etc. An implementation in the runtime system
> could easily issue all requests up front, then simultaneously wait
> for the responses. If you want to do this in SNL code, it would have
> to look like
> 
> 	for(i=0; i<2; i++)
> 		pvGet(x[i],ASYNC);
> 
> 	/* ... switch to a new state, then:*/
> 
> 	when (pvGetComplete(x[0]) && pvGetComplete(x[1])) {
> 		/* continue ... */
> 	}
> 
> which is no longer a simple loop; the code would become quite
> unwieldy, especially with more array elements.
> 
> An alternative would be to instead disallow pvGet(x) for multi-PV
> arrays (the compiler can detect this statically). This would mean you
> *have* to say
> 
> 	pvGet(x[0]);
> 
> if that is what you want to happen. I could then add new functions
> (pvGets? pvGetMultiple? pvGetMany? pvGetArray?) that will behave in
> the new way.
> 
> Similar considerations apply to other pv functions. As a side-note,
> pvPutComplete (but *not* pvGetComplete) already supports multi-PV
> arrays in a sensible way.
> 
> Comments or suggestions, especially from "sequencer power users" would
> be very much appreciated.
> 
> Cheers
> Ben
-- 
"Make it so they have to reboot after every typo." -- Scott Adams