Destructible drivers

[exzombie]

This is a followup to epics-modules/asyn#171. It adds
support to the asynNDArrayDriver and NDPluginDriver base classes, and
NDPluginROI. No changes to ADDriver are needed.

The core idea is that destructability is opt-in to maintain compatiblity with
derived classes that have not been updated yet, and may only be opted in by the
leaf class. And because one doesn't know whether any particular class is a leaf
class as it's always possible to create a derived class, destructability should
in practice be declared in the iocsh command that instantiates a driver. This is
how it was done for NDPluginROI.

To maintain compatibility with older versions of asyn, use of new symbols is
gated by #ifdef ASYN_DESTRUCTIBLE where needed. It needs to be pointed out
that virtual void shutdownPortDriver() does not need to be gated because
there's no issue with adding it even with older versions of asyn as long as you
don't use the C++11 override keyword. Only the call into the base
asynPortDriver::shutdownPortDriver() needs to be gated.

Note that I did not change the existing destruction in any way, only moved
things around. I seems to me that ~asynNDArrayDriver() is incomplete, but that
is a job for another time.

The recipe for making a driver destructible is as follows:

• If the driver does not need to be compatible with older versions of asyn
  (i.e., R44 or earlier), and is not (yet) further subclassed:

  1. Pass ASYN_DESTRUCTIBLE flag to the base constructor (i.e., NDPluginDriver
     or ADDriver).
  2. Override shutdownPortDriver() and put there code that needs to be
     executed with the driver intact. This is a good place to stop threads, for
     example. shutdownPortDriver() is a virtual function, so don't forget to
     call the base implementation.
  3. Implement the destructor. Do the cleanup as best you can. The destructors
     are actually called now, which didn't use to be the case; make use of it.
     Note that shutdownPortDriver() will only be called when the IOC shuts
     down, so, if the driver could be used outside an IOC (e.g. in unit tests),
     you should call shutdownPortDriver() from the destructor. To determine if
     it has already been run, call shutdownNeeded() which will return false
     if the shutdown has already happened.

• If the driver needs to be backwards compatible:

1. The constructor should not add ASYN_DESTRUCTIBLE to the flags. Instead,
   it should accept flags as an argument, and ASYN_DESTRUCTIBLE should be
   put there by the iocsh command that instantiates the driver. This allows
   the driver to be subclassed when the derived class is not destructible.
   Use of ASYN_DESTRUCTIBLE needs to be gated with an #ifdef.

2. Override shutdownPortDriver() and put there code that needs to run on
   IOC shutdown. shutdownPortDriver() is a virtual function, so don't
   forget to call the base implementation.

3. Implement the destructor. Note that newer versions of asyn will call it,
   but older versions will not. So, use it to release memory and such,
   but anything that needs to happen in order to disconnect from the device
   must go into shutdownPortDriver().

   Note that shutdownPortDriver() will only be called when the IOC shuts
   down, so, if the driver could be used outside an IOC (e.g. in unit tests),
   you should call shutdownPortDriver() from the destructor. To determine if
   it has already been run, you will need to set a variable in
   shudownPortDriver() yourself because the shutdownNeeded() function is only
   available in newer asyn versions.

[henriquesimoes]

To maintain compatibility with older versions of asyn, use of new symbols is
gated by #ifdef ASYN_DESTRUCTIBLE where needed. It needs to be pointed out
that virtual void shutdownPortDriver() does not need to be gated because
there's no issue with adding it even with older versions of asyn as long as you
don't use the C++11 override keyword. Only the call into the base
asynPortDriver::shutdownPortDriver() needs to be gated.

IMHO, this information should go in the commit message for d321ecf (Add port shutdown to asynNDArrayDriver, 2026-02-24) and

The recipe for making a driver destructible is as follows:
[...]

should go in the driver guidelines page (in a dedicated section?).

In general, I'm in favor of documenting any relevant aspects in each commit its message (rather than only in the PR description). This was proposed in a recent collaboration meeting, so it not a widespread practice in areaDetector repositories (yet). Do you mind rebasing your patches to add the motivation, attention points and any other relevant "why?" answers not clearly available in the patch content itself?

[exzombie]

Done! What's the approach for updating release notes? Does that belong into the PR as well?

[henriquesimoes]

Done!

Thanks!

What's the approach for updating release notes? Does that belong into the PR as well?

AFAIK, there isn't a documented procedure for this, but for most past releases @MarkRivers wrote down the notes afterwards in batches (for instance, 7ff36dc and 516e91f).

I find this process a bit sub-optimal, because we usually have a better context of the changes when they are proposed rather than when a release happens. It is also more tiresome to revisit all commits to choose which changes are relevant to be mentioned in the release notes than to decide that during the PR review.

In my opinion, we should move towards including the release notes updates in the corresponding PRs whenever possible. I'm not the one making releases, though.

[prjemian]

GitHub web has a checkbox to collate concise release notes when making a new release. Structure titles of pull requests and issues to provide content. See https://github.com/bluesky/hklpy2/releases for examples.

[prjemian]

This repo does not use GH Web for releases. Only git tags:

[MarkRivers]

In my opinion, we should move towards including the release notes updates in the corresponding PRs whenever possible.

I agree. And if the PR makes changes in the behavior or user interface then the documentation also should be updated in the PR as well.