Base asynInterpose implementation #145
Conversation
|
Very useful. Would the name 'asynInterposeTemplate' perhaps be more descriptive? |
|
Yes, that's a good idea! I will change it.. |
|
❌ Build asyn 1.0.101 failed (commit fbafabb738 by @darcato) |
|
Done! |
|
❌ Build asyn 1.0.103 failed (commit e6edd6abd9 by @darcato) |
There was a problem hiding this comment.
While I like the idea of providing a template like this with Asyn, including it in the main asyn library like you have isn't really a very good idea. Asyn is probably used by a significant proportion of all EPICS IOCs, and as written this would add a copy of this template to all of them (see my comment to your change to the asyn.dbd file).
The asyn/makeSupport directory was added as a way to provide and instantiate template code that can be modified to add new features and functionality, and I would prefer to see this code provided as a new support template type in that area, rather than including it in the main asyn library.
| int *eoslen); | ||
| static asynOctet octet = { | ||
| writeIt, readIt, flushIt, registerInterruptUser, cancelInterruptUser, | ||
| setInputEos, getInputEos, setOutputEos, getOutputEos}; |
There was a problem hiding this comment.
This octet static structure and all the routines declared static above it look wrong to me, header files shouldn't normally define concrete data structures since a new instance of that structure will be added to every object file whose source file includes that header. You seem to be expecting that only the asynInterposeTemplate.c file will include this header so the declarations belong in that .c file anyway (if any other source file tried to #include it the compiler would fail unless that source also provides definitions for all the static routines that the octet structure includes function pointers to).
| asynOctet *poctet; /* The methods we're overriding */ | ||
| void *octetPvt; /* Private data of next lower interface */ | ||
| asynUser *pasynUser; /* For connect/disconnect reporting */ | ||
| } interposePvt; |
There was a problem hiding this comment.
The interposePvt structure may be declared in the header file like this, but it will generally only be accessed by the routines that are included in the .c file so it doesn't need to be here (putting it in the header file makes it a public API). Normally the only other code which might need to know the internals of the structure would be a unit test program for the interface.
| @@ -3,6 +3,7 @@ registrar(asynInterposeFlushRegister) | |||
| registrar(asynInterposeEosRegister) | |||
| registrar(asynInterposeDelayRegister) | |||
| registrar(asynInterposeEchoRegister) | |||
| registrar(asynInterposeTemplateRegister) | |||
There was a problem hiding this comment.
The problem with including this declaration in the widely-used asyn.dbd file is that from now on every EPICS IOC that includes this DBD file will run the asynInterposeTemplateRegister() routine during its initialization and thus must include a full copy of your template interpose layer code. Please at least move the registrar() line to a separate DBD file so it isn't pulled in unnecessarily by IOCs that don't want it.
Hello,
this adds a base implementation of an
asynInterpose, which does absolutely nothing. All the requests are passed directly to the underlying port.This is useful as a guide for new users to implement a new interpose. One can just start from this base implementation and change only the functions that are useful for his use-case. Since I had to implement a custom
asynInterpose, this would have saved me a few hours.I wrote this by looking at the other
asynInterposeimplementations but I am not an expert in asyn, so let me know if I missed something.