Skip to content

Commit

Permalink
utcaApp: remove usage of sentinel parameters.
Browse files Browse the repository at this point in the history
This is a code cleanup, to avoid having to expose
UDriver::parameter_props to specific drivers.

The cleanup moves all handling of special cases (decoder_controller and
read_only) into the ParamInit class, instead of requiring passing
UDriver static inline members to it. This allowed us to add a
ParamInit::set_dc() function, which is now used by the drivers which
previously had to access parameter_props directly. This also simplifies
the UDriver constructor, since it can now determine the need for p_tmp
more easily, as well as the parameter properties.

It also allowed us to add more sanity checks, in this case to
UDriver::writeGeneral(), to report writes into read-only parameters.
  • Loading branch information
ericonr committed Nov 19, 2024
1 parent a8b5c4e commit 3d12a00
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 22 deletions.
4 changes: 1 addition & 3 deletions utcaApp/src/afc_timing.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ class AFCTiming: public UDriver {
{"CH_POL", p_decoder_controller},
{"CH_LOG", p_decoder_controller},
{"CH_ITL", p_decoder_controller},
{"CH_SRC", p_ch_src},
ParamInit{"CH_SRC", p_ch_src}.set_dc(),
{"CH_DIR", p_decoder_controller},
{"CH_PULSES", p_decoder_controller},
ParamInit{"CH_COUNT_RST", p_decoder_controller}.set_wo(),
Expand All @@ -72,8 +72,6 @@ class AFCTiming: public UDriver {
dec.set_devinfo(v);
ctl.set_devinfo(v);

parameter_props.at(p_ch_src).write_decoder_controller = true;

createParam("FREQ", asynParamFloat64, &p_freq);

read_parameters();
Expand Down
4 changes: 1 addition & 3 deletions utcaApp/src/fmcpico1m_4ch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ class FMCPico: public UDriver {
::number_of_channels,
{ },
{
{"RANGE", p_range},
ParamInit{"RANGE", p_range}.set_dc(),
},
&ctl),
dec(bars),
Expand All @@ -30,8 +30,6 @@ class FMCPico: public UDriver {
dec.set_devinfo(v);
ctl.set_devinfo(v);

parameter_props.at(p_range).write_decoder_controller = true;

read_parameters();
}

Expand Down
4 changes: 1 addition & 3 deletions utcaApp/src/rtmlamp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ class RtmLamp: public UDriver {
{"AMP_STATUS", p_read_only},
{"AMP_STATUS_LATCH", p_read_only},
{"AMP_EN", p_decoder_controller},
{"MODE", p_mode},
ParamInit{"MODE", p_mode}.set_dc(),
{"TRIG_EN", p_decoder_controller},
{"RST_LATCH", p_decoder_controller},
{"PI_KP", p_decoder_controller},
Expand All @@ -51,8 +51,6 @@ class RtmLamp: public UDriver {
dec.set_devinfo(v);
ctl.set_devinfo(v);

parameter_props.at(p_mode).write_decoder_controller = true;

read_parameters();
}

Expand Down
56 changes: 43 additions & 13 deletions utcaApp/src/udriver.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,20 +24,47 @@
#include "enumerate.h"
#include "pcie-single.h"

namespace {
struct read_only_struct {} const p_read_only;
struct decoder_controller_struct {} const p_decoder_controller;
}

struct ParamInit {
const char *name;
int &parameter;
int *parameter = nullptr;
std::optional<unsigned> number_of_channels = std::nullopt;
asynParamType type;
bool decoder_controller = false;
bool read_only = false;
bool write_only = false;

ParamInit(const char *name, int &parameter, asynParamType type = asynParamInt32):
name(name),
parameter(parameter),
parameter(&parameter),
type(type)
{
}

ParamInit(const char *name, const decoder_controller_struct &, asynParamType type = asynParamInt32):
name(name),
type(type),
decoder_controller(true)
{
}

ParamInit(const char *name, const read_only_struct &, asynParamType type = asynParamInt32):
name(name),
type(type),
read_only(true)
{
}

ParamInit set_dc()
{
decoder_controller = true;
return *this;
}

ParamInit set_nc(unsigned new_number_of_channels)
{
number_of_channels = new_number_of_channels;
Expand All @@ -59,17 +86,17 @@ class UDriver: public asynPortDriver {
static const unsigned UDRIVER_PARAMS = 2;
unsigned scan_counter = 0;

protected:
struct parameter_props {
unsigned number_of_channels;
asynParamType type;
bool is_general;
bool read_only;
bool write_only;
bool write_decoder_controller;
};
std::vector<parameter_props> parameter_props;

static inline int p_read_only = 0, p_decoder_controller = 0;
protected:
unsigned number_of_channels;
int port_number;

Expand Down Expand Up @@ -105,27 +132,23 @@ class UDriver: public asynPortDriver {
parameter_props.resize(UDRIVER_PARAMS);

auto create_params = [this](auto const &nai, bool is_general) {
for (auto &&[i, v]: enumerate(nai)) {
for (auto &[str, p, nc, t, dc, ro, wo]: nai) {
int p_tmp;
auto &&[str, p, nc, t, wo] = v;

/* if p is p_read_only or p_decoder_controller, we use a
* temporary variable to avoid any issues with simultaneous
* access to them */
bool use_tmp = &p == &p_read_only || &p == &p_decoder_controller;
int *pp = use_tmp ? &p_tmp : &p;
int *pp = p ? p : &p_tmp;
createParam(str, t, pp);

if (&p == &p_decoder_controller)
if (dc)
assert(this->generic_decoder_controller);

assert((int)parameter_props.size() == *pp);
parameter_props.push_back({
.number_of_channels = nc ? *nc : this->number_of_channels,
.type = t,
.is_general = is_general,
.read_only = ro,
.write_only = wo,
.write_decoder_controller = (&p == &p_decoder_controller),
.write_decoder_controller = dc,
});
}
};
Expand Down Expand Up @@ -285,6 +308,13 @@ class UDriver: public asynPortDriver {
if ((unsigned)function >= parameter_props.size())
return call_write_impl();

if (parameter_props.at(function).read_only) {
epicsSnprintf(pasynUser->errorMessage, pasynUser->errorMessageSize,
"writeGeneral: %s: read-only parameter", param_name);

return asynError;
}

if (parameter_props.at(function).is_general && addr != 0) {
epicsSnprintf(pasynUser->errorMessage, pasynUser->errorMessageSize,
"writeGeneral: %s: general parameter with addr=%d (should be 0)", param_name, addr);
Expand Down

0 comments on commit 3d12a00

Please sign in to comment.