epics-base / pvDataCPP

pvDataCPP is an EPICS V4 C++ module
https://epics-base.github.io/pvDataCPP/
Other
6 stars 16 forks source link

Migrate Requester, Monitor, and related types to pvAccess #35

Open mdavidsaver opened 8 years ago

mdavidsaver commented 8 years ago

As was discussed on the 15th, I'd like to migrate some classes/interfaces from the pvData* modules into pvAccess*. For the C++ code this is specifically the following types currently defined in requester.h and monitor.h.

epics::pvData::Requester
epics::pvData::RequesterPtr
epics::pvData::MessageType
epics::pvData::getMessageTypeName
epics::pvData::MonitorElement
epics::pvData::MonitorElementPtr
epics::pvData::MonitorElementPtrArray
epics::pvData::Monitor
epics::pvData::MonitorRequester

The migration plan for C++ would be to first introduce aliases into the epics::pvAccess namespace. Which allow either name to be used by dependent modules. This does not require any changes to pvDataCPP.

namespace epics { namespace pvAccess {
using epics::pvData::Requester;
}}

The next step would move the headers, and swap the namespaces and aliases. This also maintains API compatibility, but introduces potentially complex module version dependencies.

The final step is to remove the aliases from the epics::pvData namespace.

This should be split over at least 2 "major" releases. The second step could go on either side of this "split".

I'm still investigating possible ways to do a phased migration. If I (or others) can't find anything, then it would be necessary to have an API break.

mdavidsaver commented 8 years ago

I'm still investigating possible ways to do a phased migration.

... for Java

I've tried a few things (stubs with one package which extend the other) and I just don't think there is a way to do this without type aliases, which the Java language just doesn't provide.

Without aliases then eg. org.epics.pvdata.monitor.Monitor and org.epics.pvaccess.monitor.Monitor are distinct types. If one is a sub-class/interface of the other then implicit casts can work in one direction, but not both. So this approach fails either for method arguments, or return values.

dhickin commented 8 years ago

It looks to me like there are two natural groups here. First there are monitor classes

epics::pvData::MonitorElement
epics::pvData::MonitorElementPtr
epics::pvData::MonitorElementPtrArray
epics::pvData::Monitor
epics::pvData::MonitorRequester

These have always looked out of place to me in pvData. Monitoring seems to be inherently part of pvAccess and they were never used in pvData so it makes sense to move these into the pvAccess modules.

The second are the classes for messages and the requester interface.

epics::pvData::Requester
epics::pvData::RequesterPtr
epics::pvData::MessageType
epics::pvData::getMessageTypeName

Historically these were used in pvData. (Field had a message() function that sent message to a registered requester. The requester used to be used for example to print error messages in functions of the form getXXXField() like getDoubleField(). This was, though, a terrible idea). They are now not really used in pvData only in pvAccess and its downstream modules, so could move. However they don't seem so inherently pvAccess - could pvData use the interface in future? Possibly. Anyway I'm not sure the case for moving is so clear cut.

Also MessageQueue in pvData does still use the message types and includes requester.h. So you'll either have to move this too or remove it (I'm not sure anything uses it).

mdavidsaver commented 8 years ago

they don't seem so inherently pvAccess

Requester is now only used in connection with a specific remote operation. It is for this reason that I think it is naturally a part of pvAccess*. I suppose it could find use as more general logging front end, but I think this is a good fit. The fact that callers are forced to handle string formatting is inconvenient, and convenience is an important feature in a logging API.

Also MessageQueue in pvData does still use the message types

I'd never noticed MessageQueue before. It also doesn't seem to be used in pvData, but at least has some test code. My first thought is to deprecate MessageQueue as I don't like the interface. Manual ref. handling with release() is error prone. If this is undesirable, then MessageType enum could stay in pvData* or be duplicated.

mdavidsaver commented 6 years ago

As a note, this is moving forward on the C++ side. The moves have been made with compatibility typedefs for the old names.