Re: the structure of requests

[Marty K. <mrk...@co...>]

On 02/28/2012 01:11 PM, Ben Franksen wrote:
> On Tuesday, February 28, 2012, Marty Kraimer wrote:
>> Can you make the changes you suggest?
>>
>> It means:
>>
>> 1) Changing createRequest in both the Java and C++ implementation
>> 2) Changing the code in package org.epics.ioc.pvCopy.
> Sorry, Marty. I would love to but I currently do not have the time to go that
> deeply into the pvData/pvIOC development.

Well at least I tried :-(
> I can only offer suggestions at the
> moment and hope somebody picks them up and implements them.
And I do appreciate your suggestions. Please keep offering them.

> BTW, I have thought more about this, and changed my mind once again, see
> below.

I will save this for when the time comes to revisit pvCopy.

Thanks,

Marty
>> At or after the meeting at PSI you described desirable changes to pvCopy.
>> You could make these changes at the same time.
> Unfortunately it is impossible to reap the benefits of the re-factoring I had
> in mind while keeping the changes localized to pvCopy and modules that depend
> on it. After the PSI meeting I started to investigate this and I quickly
> realized that what I wanted would affect almost the whole pvData API and is
> thus not feasible at the moment.
>
> (The idea was to move the field indexing completely into the PVCopy
> implementation, so that the PVField objects would no longer have to /contain/
> an index; rather, the index would be dynamically built up during the PVCopy
> creation, i.e. whenever anyone connected to (a subset of) a structure.)
>
>> It is important that you do support the few existing conventions already
>> used in the request string passed to createRequest
>>
>> These are what shows up the pvIOCJava swtshell in the createRequest
>> field of get, put, putGet, and monitor
>>
>> get:  record[]field(value,alarm,timeStamp)
>> put:  record[]field(value)
>> putGet: record[process=true]putField(arguments)getField(result)
>> monitor: record[queueSize=2]field(value,alarm,timeStamp)
>>
>> If those conventions are still valid I think that may be all that has to
>> be changed.
> You reminded me of the thoughts I had during and after the EPICS meeting when
> I investigated possible changes to PVCopy. Let me share them, maybe they are
> helpful.
>
> Even if we separate the convenient user interface from the low-level core
> mechanism in the way I suggested, we are still mixing things that should
> better be handled separately: Field selection i.e. mapping of a specified
> subset of the fields of a source structure to a target structure need not and
> thus should not have to know anything about records. Instead of the
> complicated dynamically typed pvRequest structure, the input to the pvCopy
> creation would be a plain array of strings (or array of arrays of strings, if
> we agree to break up "name.name" before feeding the result into pvcopy
> creation).
>
> "Record options" are a mumble jumble.
>
> The process flag is the only thing here that is actually related to records
> and has IMO no business in the pvcopy object. Another class PVRecordCopy
> should be added instead that adds this as an extra argument (of type boolean)
> to the create method of its factory and the implementation of this class
> should /delegate/ the actual field mapping to a PVCopy instance that it
> contains.
>
> The queueSize option really belongs to PVCopyMonitor and should be added as an
> extra argument (of type int) to its creation.
>
> Separating all the things you have stuffed into the request into well-isolated
> independent modules / arguments would greatly simplify the implementation and
> put things where they belong.
>
> BTW, PVCopy and friends should then be moved from pvIOC to pvData.
>
> On the protocol level, I would split things along similar lines. Abolish
> pvRequest and pvRequestIF. Instead, add appropriate members to the message
> types as appropriate: all init request messages get a 'string[] fields' for
> the field selection. Monitor requests get an additional 'int queueSize'. Put
> requests (and maybe other requests, too, dependeing on whether this is
> considered to make sense) get an additional 'boolean process' flag.
>
> These changes would make everything much more transparent and easier
> understandable. It would be immediately clear which request types offer record
> processing, and which offer queueing.
>
> BTW, PVCopyFactory.create has an argument 'String structureName' that is
> documented as
>
> 	This must be one of: "field", "putField", or "getField"
>
> This suggests that the combined put-get etc requests are to be implemented on
> top of these three variants. Which makes sense, as processing may have to
> happen in between and this belongs to the PVRecord domain, not to PVStructure.
> Could you give me a pointer to where the combined put-get etc requests are
> implemented? I hope this is not in pvAccess, since that would mean the network
> layer is feature privileged over directly accessing a record (using PVCopy
> resp. my proposed PVRecordCopy).
>
> (BTW, why three variants for the mysteriously named 'structureName' argument
> i.e. why not just "putField" and "getField", i.e. what does "field" mean, and
> why use a string instead of an enum? And, most importantly, why distinguish
> these variants at all when creating the PVCopy, since the actual operation
> i.e. the three update* methods of PVCopy already specify direction of data
> flow?).
>
> Cheers
> Ben
>
> ------------------------------------------------------------------------------
> Keep Your Developer Skills Current with LearnDevNow!
> The most comprehensive online learning library for Microsoft developers
> is just $99.99! Visual Studio, SharePoint, SQL - plus HTML5, CSS3, MVC3,
> Metro Style Apps, more. Free future releases when you subscribe now!
> http://p.sf.net/sfu/learndevnow-d2d
>