Skip to content

Commit

Permalink
Improve initialization logic.
Browse files Browse the repository at this point in the history
ApplicationCore expects that each process variable that is writable from
the control-system is written once during initialization. This was
already ensured, but the way how it was done meant that this write might
happen with initial (zero) value instead of the restored value when
using Autosave.

This is improved now, so that when setting the PINI field of an output
record to YES, RUN, or RUNNING, a value that is restored through
Autosave will be the first value that is sent to the application.

In addition to that, ApplicationBase::optimiseUnmappedVariables is now
called with the set of process variables that is provided by the
application but not mapped to any EPICS records. This allows
ApplicationCore to skip updates for these PVs, improving performance.

All these changes only affect the use of the control-system adapter with
applications (through the ChimeraTK Control System Adapter). When
directly accessing a device through Device Access, these changes have no
effect.
  • Loading branch information
smarsching committed Nov 10, 2022
1 parent 93b2ab0 commit ecb22a2
Show file tree
Hide file tree
Showing 16 changed files with 349 additions and 43 deletions.
8 changes: 8 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,14 @@ automatically, even if they can be treated as numeric registers. In this case,
one of the supported data types has to be specified explicitly as part of the
register address specified in the record's `INP` or `OUT` field.

### Using Autosave

When using Autosave in order to persist the values of output records between
restarts of the IOC, please remember to set the respective records’ `PINI`
fields to `YES` (preferred), `RUN`, or `RUNNING`. Otherwise, the persited value
will be restored to the record, but the value will not be sent to the associated
application or device, so that it does not take effect.


Examples
--------
Expand Down
35 changes: 34 additions & 1 deletion chimeraTKApp/src/ChimeraTK/EPICS/ArrayRecordDeviceSupport.h
Original file line number Diff line number Diff line change
Expand Up @@ -599,7 +599,8 @@ class ArrayRecordDeviceSupport<RecordType, RecordDirection::OUTPUT>
*/
ArrayRecordDeviceSupport(RecordType *record)
: detail::ArrayRecordDeviceSupportTrait<RecordType>(record, record->out),
notifyPending(false), versionNumberValid(false), writePending(false) {
firstWritePending(false), notifyPending(false), versionNumberValid(false),
writePending(false) {
this->template callForValueTypeNoVoid<CallInitializeValue>(this);
}

Expand Down Expand Up @@ -634,6 +635,17 @@ class ArrayRecordDeviceSupport<RecordType, RecordDirection::OUTPUT>
}
};

/**
* Flag indicating that the process variable has never been written
*
* This is relevant in the context of bi-directional PVs and ensures that when
* a notification is received before the first write operation, that
* notification is discarded. This is important in order to fulfill the
* contract that we will call PVSupport::write once during initialization
* after having called PVSupport::willWrite.
*/
bool firstWritePending;

/**
* Mutex that is protecting access to notifyPending, value, versionNumber,
* versionNumberValid, and writePending.
Expand Down Expand Up @@ -730,6 +742,21 @@ class ArrayRecordDeviceSupport<RecordType, RecordDirection::OUTPUT>
// It might not always be possible to get an initial value, so it is not
// an error if this fails.
}
// If this record’s PINI field is set to YES, RUN, or RUNNING, we tell the
// PV support that we are going to call write(). This will keep it from
// calling write with a default value, thus ensuring that the first value
// that is writen is the one that has been written to this record (e.g. by
// Autosave).
switch(this->record->pini) {
case menuPiniYES:
case menuPiniRUN:
case menuPiniRUNNING:
pvSupport->willWrite();
this->firstWritePending = true;
break;
default:
break;
}
// If the PV supports notifications and the nobidirectional flag has not
// been set, we register a notification callback so that we can update the
// record's value when it changes on the device side.
Expand Down Expand Up @@ -793,6 +820,12 @@ class ArrayRecordDeviceSupport<RecordType, RecordDirection::OUTPUT>
// We have to hold a lock on the notify mutex in this function because we
// access fields that are also accessed from the notify callback.
std::lock_guard<std::recursive_mutex> lock(this->mutex);
// If the first write has not happened yet, we ensure that it happens now
// (instead of processing a notification that might have been received).
if (this->firstWritePending) {
this->firstWritePending = false;
this->notifyPending = false;
}
// We only support a fixed number of array elements, so regardless of what
// else we do, we always reset the NORD field to match NELM.
this->record->nord = this->record->nelm;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/*
* ChimeraTK control-system adapter for EPICS.
*
* Copyright 2018-2019 aquenos GmbH
* Copyright 2018-2022 aquenos GmbH
*
* The ChimeraTK Control System Adapter for EPICS is free software: you can
* redistribute it and/or modify it under the terms of the GNU Lesser General
Expand Down Expand Up @@ -72,6 +72,9 @@ class ControlSystemAdapterPVProvider :
*/
virtual ~ControlSystemAdapterPVProvider();

// Declared in PVProvider.
virtual void finalizeInitialization() override;

// Declared in PVProvider.
virtual std::type_info const &getDefaultType(
std::string const &processVariableName) override;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/*
* ChimeraTK control-system adapter for EPICS.
*
* Copyright 2018-2019 aquenos GmbH
* Copyright 2018-2022 aquenos GmbH
*
* The ChimeraTK Control System Adapter for EPICS is free software: you can
* redistribute it and/or modify it under the terms of the GNU Lesser General
Expand Down Expand Up @@ -122,6 +122,9 @@ class ControlSystemAdapterPVSupport : public PVSupport<T> {
ReadCallback const &successCallback,
ErrorCallback const &errorCallback) override;

// Declared in PVSupport.
virtual void willWrite() override;

// Declared in PVSupport.
virtual bool write(Value const &value,
VersionNumber const &versionNumber,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/*
* ChimeraTK control-system adapter for EPICS.
*
* Copyright 2018-2019 aquenos GmbH
* Copyright 2018-2022 aquenos GmbH
*
* The ChimeraTK Control System Adapter for EPICS is free software: you can
* redistribute it and/or modify it under the terms of the GNU Lesser General
Expand Down Expand Up @@ -150,6 +150,11 @@ bool ControlSystemAdapterPVSupport<T>::read(
return this->shared->read(successCallback, errorCallback);
}

template<typename T>
void ControlSystemAdapterPVSupport<T>::willWrite() {
this->shared->willWrite();
}

template<typename T>
bool ControlSystemAdapterPVSupport<T>::write(
Value const &value,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/*
* ChimeraTK control-system adapter for EPICS.
*
* Copyright 2018-2020 aquenos GmbH
* Copyright 2018-2022 aquenos GmbH
*
* The ChimeraTK Control System Adapter for EPICS is free software: you can
* redistribute it and/or modify it under the terms of the GNU Lesser General
Expand Down Expand Up @@ -88,6 +88,15 @@ class ControlSystemAdapterSharedPVSupportBase {
return this->index;
}

/**
* Calls the underlying ProcessArray’s write() method, but only if willWrite()
* has not been called.
*
* This ensures that every process variable is written once in the
* initialization phase, as required by the ApplicationCore specification.
*/
virtual void initialWriteIfNeeded() = 0;

/**
* Tells whether doNotify() may be called. The PVProvider will only call
* doNotify() when this method returns true. Otherwise, it will periodically
Expand Down Expand Up @@ -219,6 +228,15 @@ class ControlSystemAdapterSharedPVSupport
ReadCallback const &successCallback,
ErrorCallback const &errroCallback);

/**
* Called to indicate that the process variable is going to be written during
* the startup phase.
*
* We use this in initialWriteIfNeeded() in order to decide whether we need to
* write the underlying process variable once.
*/
void willWrite();

/**
* Writes a value and calls the callback.
*
Expand Down Expand Up @@ -246,6 +264,9 @@ class ControlSystemAdapterSharedPVSupport
// Declared in ControlSystemAdapterSharedPVSupportBase.
virtual std::function<void()> doNotify() override;

// Declared in ControlSystemAdapterSharedPVSupportBase.
virtual void initialWriteIfNeeded() override;

// Declared in ControlSystemAdapterSharedPVSupportBase.
virtual bool readyForNextNotification() override;

Expand Down Expand Up @@ -321,6 +342,13 @@ class ControlSystemAdapterSharedPVSupport
*/
std::forward_list<std::weak_ptr<ControlSystemAdapterPVSupport<T>>> pvSupports;

/**
* Flag indicating whether at least one of the records that uses this PV
* support is going to call write() during the initialization phase of the
* IOC.
*/
bool willWriteCalled;

/**
* Calls the specified notify callback with the current value of the PV. This
* is guaranteed to happen before notifying it with a regular notification.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ ControlSystemAdapterSharedPVSupport<T>::ControlSystemAdapterSharedPVSupport(
std::string const &name, std::size_t index)
: ControlSystemAdapterSharedPVSupportBase(index),
mutex(pvProvider->mutex), name(name), notificationPendingCount(0),
notifyCallbackCount(0), pvProvider(pvProvider) {
notifyCallbackCount(0), pvProvider(pvProvider), willWriteCalled(false) {
{
std::lock_guard<std::recursive_mutex> lock(this->mutex);
this->processArray = this->pvProvider->pvManager
Expand Down Expand Up @@ -142,6 +142,12 @@ bool ControlSystemAdapterSharedPVSupport<T>::read(
return true;
}

template<typename T>
void ControlSystemAdapterSharedPVSupport<T>::willWrite() {
std::lock_guard<std::recursive_mutex> lock(this->mutex);
this->willWriteCalled = true;
}

template<typename T>
bool ControlSystemAdapterSharedPVSupport<T>::write(
Value const &value,
Expand Down Expand Up @@ -266,6 +272,25 @@ std::function<void()> ControlSystemAdapterSharedPVSupport<T>::doNotify() {
};
}

template<typename T>
void ControlSystemAdapterSharedPVSupport<T>::initialWriteIfNeeded() {
std::lock_guard<std::recursive_mutex> lock(this->mutex);
// If this process variable is not writable or write is going to be called by
// a record, we do not have to do anything here.
if (!this->canWrite() || this->willWriteCalled) {
return;
}
// If write is not going to be called, we call it here, so that it is called
// once during initialization as required by the specification at
// https://chimeratk.github.io/ApplicationCore/master/spec_initial_value_propagation.html.
// We do not use the version number supplied by initialValue(). Instead, we
// generate a new one. The initial version number might be the null version
// number, and we are not supposed to use that one in any transfer operation.
Value initialValue;
std::tie(initialValue, std::ignore) = this->initialValue();
write(std::move(initialValue), VersionNumber(), nullptr, nullptr);
}

template<typename T>
bool ControlSystemAdapterSharedPVSupport<T>::readyForNextNotification() {
// We are ready to deliver the next notification when the last notifications
Expand Down
7 changes: 6 additions & 1 deletion chimeraTKApp/src/ChimeraTK/EPICS/ConvertingPVSupport.h
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/*
* ChimeraTK control-system adapter for EPICS.
*
* Copyright 2018 aquenos GmbH
* Copyright 2018-2022 aquenos GmbH
*
* The ChimeraTK Control System Adapter for EPICS is free software: you can
* redistribute it and/or modify it under the terms of the GNU Lesser General
Expand Down Expand Up @@ -159,6 +159,11 @@ class ConvertingPVSupport : public PVSupport<TargetType> {
return originalPVSupport->read(wrappedSuccessCallback, wrappedErrorCallback);
}

// Declared in PVSupport.
void willWrite() override {
originalPVSupport->willWrite();
}

// Declared in PVSupport.
bool write(Value const &value,
WriteCallback const &successCallback,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ extern "C" {
#include <callback.h>
#include <dbLink.h>
#include <dbScan.h>
#include <menuPini.h>
} // extern "C"

#include "RecordDeviceSupportBase.h"
Expand Down Expand Up @@ -525,7 +526,8 @@ class FixedScalarRecordDeviceSupport<RecordType, RecordDirection::OUTPUT, ValueF
FixedScalarRecordDeviceSupport(RecordType *record)
: detail::FixedScalarRecordDeviceSupportTrait<RecordType, ValueFieldName>(
record, record->out),
notifyPending(false), versionNumberValid(false), writePending(false) {
firstWritePending(false), notifyPending(false), versionNumberValid(false),
writePending(false) {
this->template callForValueType<CallInitializeValue>(this);
}

Expand Down Expand Up @@ -600,6 +602,17 @@ class FixedScalarRecordDeviceSupport<RecordType, RecordDirection::OUTPUT, ValueF
struct CallProcessInternal
: CallProcessInternalTemplate<T, void> {};

/**
* Flag indicating that the process variable has never been written
*
* This is relevant in the context of bi-directional PVs and ensures that when
* a notification is received before the first write operation, that
* notification is discarded. This is important in order to fulfill the
* contract that we will call PVSupport::write once during initialization
* after having called PVSupport::willWrite.
*/
bool firstWritePending;

/**
* Mutex that is protecting access to notifyPending, value, versionNumber,
* versionNumberValid, and writePending.
Expand Down Expand Up @@ -686,6 +699,21 @@ class FixedScalarRecordDeviceSupport<RecordType, RecordDirection::OUTPUT, ValueF
// It might not always be possible to get an initial value, so it is not
// an error if this fails.
}
// If this record’s PINI field is set to YES, RUN, or RUNNING, we tell the
// PV support that we are going to call write(). This will keep it from
// calling write with a default value, thus ensuring that the first value
// that is writen is the one that has been written to this record (e.g. by
// Autosave).
switch(this->record->pini) {
case menuPiniYES:
case menuPiniRUN:
case menuPiniRUNNING:
pvSupport->willWrite();
this->firstWritePending = true;
break;
default:
break;
}
// If the PV supports notifications and the nobidirectional flag has not
// been set, we register a notification callback so that we can update the
// record's value when it changes on the device side.
Expand Down Expand Up @@ -749,6 +777,12 @@ class FixedScalarRecordDeviceSupport<RecordType, RecordDirection::OUTPUT, ValueF
// We have to hold a lock on the notify mutex in this function because we
// access fields that are also accessed from the notify callback.
std::lock_guard<std::recursive_mutex> lock(this->mutex);
// If the first write has not happened yet, we ensure that it happens now
// (instead of processing a notification that might have been received).
if (this->firstWritePending) {
this->firstWritePending = false;
this->notifyPending = false;
}
// If the record's PACT field is set, this method is called because an
// asynchronous read completed.
if (this->record->pact) {
Expand Down
18 changes: 17 additions & 1 deletion chimeraTKApp/src/ChimeraTK/EPICS/PVProvider.h
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/*
* ChimeraTK control-system adapter for EPICS.
*
* Copyright 2018 aquenos GmbH
* Copyright 2018-2022 aquenos GmbH
*
* The ChimeraTK Control System Adapter for EPICS is free software: you can
* redistribute it and/or modify it under the terms of the GNU Lesser General
Expand Down Expand Up @@ -56,11 +56,27 @@ class PVProvider {
*
* If there is no process variable with the specified name, this method throws
* an exception.
*
* If this method is called after finalizeInitialization(), it may throw an
* exception.
*/
template<typename ElementType>
typename PVSupport<ElementType>::SharedPtr createPVSupport(
std::string const &processVariableName);

/**
* Finalizes the initialization of this PV provider.
*
* This is called after all the needed PV supports have been created.
* Implementations may override this to carry out any initialization logic
* that needs to be run after PV supports have been created but before general
* operation of the EPICS IOC commences.
*
* This method may throw an exception if it is called more than once.
*/
virtual void finalizeInitialization() {
}

/**
* Returns the default type for the specified process variable. The default
* type is always compatible with the process variable. This means that it is
Expand Down
Loading

0 comments on commit ecb22a2

Please sign in to comment.