rossvideo / Catena

Other
4 stars 0 forks source link

get/set value support is incomplete in C++ SDK #102

Closed mejohnnaylor closed 3 months ago

mejohnnaylor commented 4 months ago

The C++ SDK includes these means to set/get values in the device model (DM).

See in ParamAccessor.h the type aliases for: Setter - used by service to set values in DM Getter - used by service to get values from DM SetterAt - used by service to set values in arrays in DM GetterAt - used by service to get values in arrays in DM

VariantInfoGetter - used by the REFLECTABLE_VARIANT macro to serialize between std::variant<> and the DM

ValueGetter - used by both clients* and service to get catena::Value objects from the DM ValueSetter - used by clients to set values in the MD

*by clients I mean the RPC handlers working on behalf of clients. The point is they're making writes to the DM on behalf of clients.

Each of these type aliases to a "Functory" which is a portmanteau of "Function Factory". Basically there's a look-up mechanism to return a function based on the look-up value that will make the right calls on a polymorphic catena::Value object based on the parameter type.

Not all the Functories have complete support for all types.

The ParamAccessor constructor is where all the access functions should be registered with all the functories.

        // get our functories
        auto &setter = Setter::getInstance();

        ...

        // register int32 setter
        setter.addFunction(KindCase::kInt32Value, [](Value *dst, const void *srcAddr) {
            dst->set_int32_value(*reinterpret_cast<const int32_t *>(srcAddr));
        });

is an excerpt from ParamAccessor.cpp that shows the Setter functor being instantiated, and then the function to set int32s being registered with it.

The task is to 1) inventory what types are already supported for all 6 Functories (with the possible exclusion of the VariantInfo one)

This is basically a 2d grid:

Functory INT32 FLOAT STRING STRUCT VARIANT INT32 ARRAY FLOAT ARRAY STRUCT ARRAY VARIANT ARRAY
Getter
Setter
GetterAt NA NA NA NA NA
SetterAt NA NA NA NA NA
ValueGetter
ValueSetter

2) complete support for the missing items, starting with the easy ones (i.e. not STRUCT or VARIANT first) 3) update the one_of_everything DM to include, well, every parameter type 4) use this DM to verify correct operations 5) submit PR, walk thru code, discuss how to approach the STRUCT and VARIANT work 6) repeat 2 - 5 for STRUCT 7) repeat 2 - 5 for VARIANT

mejohnnaylor commented 4 months ago

extra credit for writing macros to do this - you can see how repetitive the code is with only minor changes between each setter & getter. DRY (don't repeat yourself) is an important principle in software engineering. Macros similar to:

#define SETTER_INSTANCE(kind_case, set_method, cast) \
setter.addFunction( kind_case, [](Value *dst, const void *srcAddr) { \
  dst->set_method(*reinterpret_cast<cast*>(srcAddr)); \
})

// and then...
SETTER_INSTANCE(KindCase::kInt32Value, set_int32_value, int32_t);
SETTER_INSTANCE(KindCAse::kFloat32Value, set_float32_value, float);

// ... etc
johndanenRV commented 4 months ago

Here is the table of the functions that have already been implemented.

image

mejohnnaylor commented 4 months ago

all simple types covered, need to tackle struct & variants