Conversation
Member
A bit :)
Either. The place to start is why? What are you trying to accomplish? Why your specific proposed change? And why are the existing plugin mechanisms in epics-base insufficient? |
Member
|
Also, to give fair warning. I have strong views on the advisability of minimal and well defined interfaces. The idea of making the source tree of PVXS a plugin interface really does not appeal to me. This particular approach looks likely to be a very hard sell. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
As always with this, I have no idea if I'm following the correct etiquette. This seems too big to spring out of the blue (and it started so much smaller...). Should I have started this as an issue? A discussion via email or mailing list? Do let me know how to do these better!
Site Extension Framework for PVXS IOC
Adds a lightweight plugin system that lets facilities drop
.cppfiles into asite/directory and have them compiled automatically intolibpvxsIoc. This allows customisation without modifying any upstream source files. As a demonstration of the API, theQ:time:tag/utagfeature is refactored out of the core and reimplemented as a site extension.The primary development goal of the new extension system was to minimise changes to existing PVXS code. Most changes in the
ioc/directory aren't directly due to the new site extension system, but instead due to the example of movingQ:time:taginto the new framework. Unfortunately, necessary changes to the build system make this less obvious.The primary development motivation for this framework is to support different alarm messages (see PR #153 ). The proposed site extension framework allows this without needing any alteration of PVXS base or any mention of alarms at all!
Changes
1. Site extension framework
Affected files: +
ioc/sitehooks.h, +ioc/sitehooks.cpp,ioc/iochooks.cpp,ioc/iocsource.cppA new
pvxs::ioc::sitenamespace provides a (PVXS_IOC_API) API of three registration functions, declared inioc/sitehooks.h. For each registration function, multiple callbacks may be added and fire in registration order.addInitHookAtBeginning(fn)void()callback fired atinitHookAtBeginning. Used to reset per-IOC state (e.g. during testing) and to scan static database configuration such as info fields.addInitHookAfterIocBuilt(fn)void()callback fired atinitHookAfterIocBuilt. Used to read field values or follow links that are only valid after full initialisation.addNodePostProcessor(fn)void(dbCommon*, Value&)callback fired at the end of everyIOCSource::get()after all standard fields have been populated. Allows fields to be modified before a PVA response is sent to a client.Adding a
.cppfile to the newsite/directory automatically adds it as an extension into the IOC.site::registerHooks()is added topvxsBaseRegistrar()inioc/iochooks.cpp. It callsregisterSiteExtensions()insite/siteregister.cpp, which is generated at build time bysite/gen_siteregister.pyfrom the list ofsite/*.cppfiles (scanned by theMakefileorsetup.py). For each file matching the naming convention (e.g.foo.cpp->registerFoo()), a call to its registrar function is added toregisterSiteExtensions().ioc/iocsource.cppcallssite::postProcessNode()at the end ofIOCSource::get(), giving registered post-processors the opportunity to read or overwrite any field in the PVA response node.Build system and packaging
Affected files:
Makefile,setup.py,MANIFEST.in,ioc/Makefile,site/Makefile,site/gen_siteregister.pyIn the top-level project
Makefile,site/is added as a build directory beforeioc/so thatgen_siteregister.pyproducessite/siteregister.cppbefore theioc/build needs it.site/test/is added as a test directory afterioc/.The Python/pip build path (
setup.py) is kept in sync with the Makefile changes. It invokesgen_siteregister.pyat build time through a newgenerate_siteregister()call.ioc/sitehooks.cppis added to the shared library sources, and globssite/*.cppinto the same list.The
ioc/Makefileis altered so thatsite/*.cppfiles are compiled intolibpvxsIoc.MANIFEST.inis extended to includesite/*.cpp,site/*.h, andsite/gen_siteregister.pyin the Python source distribution tarball.Test organisation
Affected files:
site/test/MakefileTests for site extensions live in
site/test/, separate from the coretest/directory. Thesite/test/Makefilefollows a naming convention to run test binaries automatically: for each<name>.cppinsite/, if a matchingtest<name>.cppexists insite/test/, atest<name>binary is built from those two files plus the standard IOC driver registration stub. If a matchingtest<name>.dbfile also exists it is added toTESTFILESautomatically. Extensions that need extra source files, extra libraries, or a non-standard binary name must be added manually below the auto-discovery block.Each test binary links against
pvxsIocand therefore exercises the extension through the full hook registration path, the same as a real IOC. Tests useTestIOCanddbUnitTest.h, so they require EPICS Base >= 3.15; thesite/test/build is skipped on older bases via aBASE_3_15guard.2.
Q:time:tagmoved from core to site extensionAffected files:
ioc/fieldconfig.h,ioc/typeutils.cpp,ioc/field.cpp,ioc/singlesrcsubscriptionctx.h,ioc/iocsource.cpp,site/timetag.cppThe
nsecMaskfield andupdateNsecMask()method have been removed fromMappingInfo. TheQ:time:tag/nsec:lsb:Nfeature is now implemented insite/timetag.cppusingaddNodePostProcessor, which has access to the same record pointer and PVA node.This keeps the core
IOCSource::get()path free of facility-specific logic and demonstrates that the post-processor hook is sufficient to modify PVA fields.Test suite cleanup
Affected files:
test/testqsingle.cpp,test/testqsingle.dbThe
test:nsecrecord and its associatedQ:time:tagassertions have been removed fromtestqsingle, since that feature now lives insite/timetag.cppand is tested insite/test/testtimetag.cpp.3. Other minor changes
codespell.dicaddsstaticsto the spell-check whitelist, due to a comment insitehooks.cpp.Site directory
The
site/directory includes thetimetagextension but should work with no.cppfiles present. Facilities may add their own.cppfiles alongside or instead oftimetag.cpp. AREADME.mddescribes the API and gives some guidance on how to add a new extension.Comments and Questions
I've used the
pvxs::ioc::sitenamespace to make it the join between the base and extension code as clear as possible.It would be possible to update
pvxinfo -Dto include information about the extensions added (using an addition to the existinggen_siteregister.pyautomated generation step) so that it's possible to see that site extensions might be the source of reported issues.Should
sitehooks.cppbe usingPVXS_EXPERT_API_ENABLED?