diff --git a/lib/pycsh_core/meson.build b/lib/pycsh_core/meson.build index c77a282..7c791a7 100644 --- a/lib/pycsh_core/meson.build +++ b/lib/pycsh_core/meson.build @@ -18,11 +18,9 @@ pycsh_sources = [ # Classes 'src/parameter/parameter.c', - 'src/parameter/parameterarray.c', 'src/parameter/pythonparameter.c', - 'src/parameter/pythonarrayparameter.c', - 'src/parameter/pythongetsetparameter.c', - 'src/parameter/pythongetsetarrayparameter.c', + 'src/parameter/dynamicparameter.c', + 'src/parameter/getsetparameter.c', 'src/parameter/parameterlist.c', 'src/csp_classes/ident.c', 'src/csp_classes/vmem.c', diff --git a/lib/pycsh_core/pycsh.pyi b/lib/pycsh_core/pycsh.pyi index 15564ba..2c87bbb 100644 --- a/lib/pycsh_core/pycsh.pyi +++ b/lib/pycsh_core/pycsh.pyi @@ -278,11 +278,11 @@ class ParameterArray(Parameter): """ -class PythonParameter(Parameter): +class DynamicParameter(Parameter): """ Parameter created in Python. """ def __new__(cls, id: int, name: str, type: int, mask: int | str, unit: str = None, docstr: str = None, array_size: int = 0, - callback: _Callable[[Parameter, int], None] = None, host: int = None, timeout: int = None, + callback: _Callable[[Parameter, int], None] = None, host: int = None, node: int = None, timeout: int = None, retries: int = 0, paramver: int = 2) -> PythonParameter: """ Create a new parameter from the provided options, and add it to the global list. @@ -295,13 +295,37 @@ class PythonParameter(Parameter): :param docstr: Docstring of the new parameter. :param array_size: Array size of the new parameter. Creates a ParameterArray when > 1. :param callback: Python function called when the parameter is set, signature: def callback(param: Parameter, offset: int) -> None - :param host: + :param node: 0 for local parameter, otherwise declares a remote parameter. :param timeout: Timeout to use when setting remote parameters. :param retries: Amount of retries when setting remote parameters. :param paramver: Parameter version to use for this parameter. :return: The created Parameter instance. """ + def list_add(self, override: bool = False) -> DynamicParameter: + """ + Adds `self` to the global parameter list, + making it available to others on the network. + + Will never error if `self` is already in the list. + + :param override: How to handle an existing overlapping parameter in the list: + False = Raise exception + True = Override/Replace + + :returns: self or an existing parameter with overlapping node and ID + """ + + def list_forget(self) -> bool: + """ + Removes `self` from the global parameter list. + + Will not remove overlapping parameters, + those should first be found using Parameter() + + :returns: True if `self` was removed from the list, False if `self` was not found in the list. + """ + @property def keep_alive(self) -> bool: """ @@ -329,10 +353,10 @@ class PythonParameter(Parameter): Change the callback of the parameter """ -class PythonArrayParameter(PythonParameter, ParameterArray): - """ ParameterArray created in Python. """ +class PythonParameter(DynamicParameter): + """ Parameter created in Python. """ -class PythonGetSetParameter(PythonParameter): +class GetSetParameter(PythonParameter): """ ParameterArray created in Python. """ def __new__(cls, id: int, name: str, type: int, mask: int | str, unit: str = None, docstr: str = None, array_size: int = 0, @@ -340,9 +364,6 @@ class PythonGetSetParameter(PythonParameter): retries: int = 0, paramver: int = 2, getter: _Callable[[Parameter, int], _Any] = None, setter: _Callable[[Parameter, int, _Any], None] = None) -> PythonGetSetParameter: """ """ -class PythonGetSetArrayParameter(PythonGetSetParameter, PythonArrayParameter): - """ ParameterArray created in Python. """ - # PyCharm may refuse to acknowledge that a list subclass is iterable, so we explicitly state that it is. class ParameterList(_pylist[Parameter | ParameterArray], _Iterable): """ diff --git a/lib/pycsh_core/src/parameter/dynamicparameter.c b/lib/pycsh_core/src/parameter/dynamicparameter.c new file mode 100644 index 0000000..d6a68f9 --- /dev/null +++ b/lib/pycsh_core/src/parameter/dynamicparameter.c @@ -0,0 +1,515 @@ +/* + * dynamicparameter.c + * + * Contains the DynamicParameter Parameter subclass. + * + */ + +#include "dynamicparameter.h" + +// It is recommended to always define PY_SSIZE_T_CLEAN before including Python.h +#define PY_SSIZE_T_CLEAN +#include + +#include "structmember.h" + +#include + +#include "../pycsh.h" +#include "../utils.h" + +// Instantiated in our PyMODINIT_FUNC +PyObject * PyExc_ParamCallbackError; +PyObject * PyExc_InvalidParameterTypeError; + +/** + * @brief Shared callback for all param_t's wrapped by a Parameter instance. + */ +void Parameter_callback(param_t * param, int offset) { + PyGILState_STATE CLEANUP_GIL gstate = PyGILState_Ensure(); + assert(Parameter_wraps_param(param)); + assert(!PyErr_Occurred()); // Callback may raise an exception. But we don't want to override an existing one. + + PyObject *key AUTO_DECREF = PyLong_FromVoidPtr(param); + DynamicParameterObject *python_param = (DynamicParameterObject*)PyDict_GetItem((PyObject*)param_callback_dict, key); + + /* This param_t uses the Python Parameter callback, but doesn't actually point to a Parameter. + Perhaps it was deleted? Or perhaps it was never set correctly. */ + if (python_param == NULL) { + assert(false); // TODO Kevin: Is this situation worthy of an assert(), or should we just ignore it? + return; + } + + // DynamicParameterObject *python_param = (DynamicParameterObject *)((char *)param - offsetof(DynamicParameterObject, parameter_object.param)); + PyObject *python_callback = python_param->callback; + + /* This Parameter has no callback */ + /* Python_callback should not be NULL here when Parameter_wraps_param(), but we will allow it for now... */ + if (python_callback == NULL || python_callback == Py_None) { + return; + } + + assert(PyCallable_Check(python_callback)); + /* Create the arguments. */ + PyObject *pyoffset AUTO_DECREF = Py_BuildValue("i", offset); + PyObject * args AUTO_DECREF = PyTuple_Pack(2, python_param, pyoffset); + /* Call the user Python callback */ + PyObject *value AUTO_DECREF = PyObject_CallObject(python_callback, args); + + if (PyErr_Occurred()) { + /* It may not be clear to the user, that the exception came from the callback, + we therefore chain unto the existing exception, for better clarity. */ + /* _PyErr_FormatFromCause() seems simpler than PyException_SetCause() and PyException_SetContext() */ + // TODO Kevin: It seems exceptions raised in the CSP thread are ignored. + _PyErr_FormatFromCause(PyExc_ParamCallbackError, "Error calling Python callback"); + #if PYCSH_HAVE_APM // TODO Kevin: This is pretty ugly, but we can't let the error propagate when building for APM, as there is no one but us to catch it. + /* It may not be clear to the user, that the exception came from the callback, + we therefore chain unto the existing exception, for better clarity. */ + /* _PyErr_FormatFromCause() seems simpler than PyException_SetCause() and PyException_SetContext() */ + // TODO Kevin: It seems exceptions raised in the CSP thread are ignored. + //_PyErr_FormatFromCause(PyExc_ParamCallbackError, "Error calling Python callback"); + PyErr_Print(); + #endif + } +} + +// Source: https://chat.openai.com +/** + * @brief Check that the callback accepts exactly one Parameter and one integer, + * as specified by "void (*callback)(struct param_s * param, int offset)" + * + * Currently also checks type-hints (if specified). + * Additional optional arguments are also allowed, + * as these can be disregarded by the caller. + * + * @param callback function to check + * @param raise_exc Whether to set exception message when returning false. + * @return true for success + */ +bool is_valid_callback(const PyObject *callback, bool raise_exc) { + + /*We currently allow both NULL and Py_None, + as those are valid to have on DynamicParameterObject */ + if (callback == NULL || callback == Py_None) + return true; + + // Suppress the incompatible pointer type warning when AUTO_DECREF is used on subclasses of PyObject* + #pragma GCC diagnostic push + #pragma GCC diagnostic ignored "-Wincompatible-pointer-types" + + // Get the __code__ attribute of the function, and check that it is a PyCodeObject + // TODO Kevin: Hopefully it's safe to assume that PyObject_GetAttrString() won't mutate callback + PyCodeObject *func_code AUTO_DECREF = (PyCodeObject*)PyObject_GetAttrString((PyObject*)callback, "__code__"); + if (!func_code || !PyCode_Check(func_code)) { + if (raise_exc) + PyErr_SetString(PyExc_TypeError, "Provided callback must be callable"); + return false; + } + + int accepted_pos_args = pycsh_get_num_accepted_pos_args(callback, raise_exc); + if (accepted_pos_args < 2) { + if (raise_exc) + PyErr_SetString(PyExc_TypeError, "Provided callback must accept at least 2 positional arguments"); + return false; + } + + // Check for too many required arguments + int num_non_default_pos_args = pycsh_get_num_required_args(callback, raise_exc); + if (num_non_default_pos_args > 2) { + if (raise_exc) + PyErr_SetString(PyExc_TypeError, "Provided callback must not require more than 2 positional arguments"); + return false; + } + + // Get the __annotations__ attribute of the function + // TODO Kevin: Hopefully it's safe to assume that PyObject_GetAttrString() won't mutate callback + PyDictObject *func_annotations AUTO_DECREF = (PyDictObject *)PyObject_GetAttrString((PyObject*)callback, "__annotations__"); + + // Re-enable the warning + #pragma GCC diagnostic pop + + assert(PyDict_Check(func_annotations)); + if (!func_annotations) { + return true; // It's okay to not specify type-hints for the callback. + } + + // Get the parameters annotation + // PyCode_GetVarnames() exists and should be exposed, but it doesn't appear to be in any visible header. + PyObject *param_names AUTO_DECREF = PyObject_GetAttrString((PyObject*)func_code, "co_varnames");// PyCode_GetVarnames(func_code); + if (!param_names) { + return true; // Function parameters have not been annotated, this is probably okay. + } + + // Check if it's a tuple + if (!PyTuple_Check(param_names)) { + // TODO Kevin: Not sure what exception to set here. + if (raise_exc) + PyErr_Format(PyExc_TypeError, "param_names type \"%s\" %p", param_names->ob_type->tp_name, param_names); + return false; // Not sure when this fails, but it's probably bad if it does. + } + + PyObject *typing_module_name AUTO_DECREF = PyUnicode_FromString("typing"); + if (!typing_module_name) { + return false; + } + + PyObject *typing_module AUTO_DECREF = PyImport_Import(typing_module_name); + if (!typing_module) { + if (raise_exc) + PyErr_SetString(PyExc_ImportError, "Failed to import typing module"); + return false; + } + +#if 1 + PyObject *get_type_hints AUTO_DECREF = PyObject_GetAttrString(typing_module, "get_type_hints"); + if (!get_type_hints) { + if (raise_exc) + PyErr_SetString(PyExc_ImportError, "Failed to get 'get_type_hints()' function"); + return false; + } + assert(PyCallable_Check(get_type_hints)); + + + PyObject *type_hint_dict AUTO_DECREF = PyObject_CallFunctionObjArgs(get_type_hints, callback, NULL); + +#else + + PyObject *get_type_hints_name AUTO_DECREF = PyUnicode_FromString("get_type_hints"); + if (!get_type_hints_name) { + return false; + } + + PyObject *type_hint_dict AUTO_DECREF = PyObject_CallMethodObjArgs(typing_module, get_type_hints_name, callback, NULL); + if (!type_hint_dict) { + if (raise_exc) + PyErr_SetString(PyExc_ImportError, "Failed to get type hints of callback"); + return false; + } +#endif + + // TODO Kevin: Perhaps issue warnings for type-hint errors, instead of errors. + { // Checking first parameter type-hint + + // co_varnames may be too short for our index, if the signature has *args, but that's okay. + if (PyTuple_Size(param_names)-1 <= 0) { + return true; + } + + PyObject *param_name = PyTuple_GetItem(param_names, 0); + if (!param_name) { + if (raise_exc) + PyErr_SetString(PyExc_IndexError, "Could not get first parameter name"); + return false; + } + + PyObject *param_annotation = PyDict_GetItem(type_hint_dict, param_name); + if (param_annotation != NULL && param_annotation != Py_None) { + if (!PyType_Check(param_annotation)) { + if (raise_exc) + PyErr_Format(PyExc_TypeError, "First parameter annotation is %s, which is not a type", param_annotation->ob_type->tp_name); + return false; + } + if (!PyObject_IsSubclass(param_annotation, (PyObject *)&ParameterType)) { + if (raise_exc) + PyErr_Format(PyExc_TypeError, "First callback parameter should be type-hinted as Parameter (or subclass). (not %s)", param_annotation->ob_type->tp_name); + return false; + } + } + } + + { // Checking second parameter type-hint + + // co_varnames may be too short for our index, if the signature has *args, but that's okay. + if (PyTuple_Size(param_names)-1 <= 1) { + return true; + } + + PyObject *param_name = PyTuple_GetItem(param_names, 1); + if (!param_name) { + if (raise_exc) + PyErr_SetString(PyExc_IndexError, "Could not get first parameter name"); + return false; + } + + PyObject *param_annotation = PyDict_GetItem(type_hint_dict, param_name); + if (param_annotation != NULL && param_annotation != Py_None) { + if (!PyType_Check(param_annotation)) { + if (raise_exc) + PyErr_Format(PyExc_TypeError, "Second parameter annotation is %s, which is not a type", param_annotation->ob_type->tp_name); + return false; + } + if (!PyObject_IsSubclass(param_annotation, (PyObject *)&PyLong_Type)) { + if (raise_exc) + PyErr_Format(PyExc_TypeError, "Second callback parameter should be type-hinted as int offset. (not %s)", param_annotation->ob_type->tp_name); + return false; + } + } + } + + return true; +} + +static void DynamicParameter_dealloc(DynamicParameterObject *self) { + + if (self->callback != NULL && self->callback != Py_None) { + Py_XDECREF(self->callback); + self->callback = NULL; + } + + /* We defer deallocation to our Parameter baseclass, + as it must also handle deallocation of param_t's that have been "list forget"en anyway. */ + param_list_remove_specific(((ParameterObject*)self)->param, 0, 0); + + PyTypeObject *baseclass = pycsh_get_base_dealloc_class(&DynamicParameterType); + baseclass->tp_dealloc((PyObject*)self); +} + +static int Parameter_set_callback(DynamicParameterObject *self, PyObject *value, void *closure) { + + if (value == NULL) { + PyErr_SetString(PyExc_TypeError, "Cannot delete the callback attribute"); + return -1; + } + + if (!is_valid_callback(value, true)) { + return -1; + } + + if (value == self->callback) + return 0; // No work to do + + /* Changing the callback to None. */ + if (value == Py_None) { + if (self->callback != Py_None) { + /* We should not arrive here when the old value is Py_None, + but prevent Py_DECREF() on it at all cost. */ + Py_XDECREF(self->callback); + } + self->callback = Py_None; + return 0; + } + + /* We now know that 'value' is a new callable. */ + + /* When replacing a previous callable. */ + if (self->callback != Py_None) { + Py_XDECREF(self->callback); + } + + Py_INCREF(value); + self->callback = value; + + return 0; +} + +/* Internal API for creating a new DynamicParameterObject. */ +__attribute__((malloc(DynamicParameter_dealloc, 1))) +DynamicParameterObject * Parameter_create_new(PyTypeObject *type, uint16_t id, uint16_t node, param_type_e param_type, uint32_t mask, char * name, char * unit, char * docstr, int array_size, const PyObject * callback, int host, int timeout, int retries, int paramver) { + + /* Check for valid parameter type. param_list_create_remote() should always return NULL for errors, + but this allows us to raise a specific exception. */ + /* I'm not sure whether we can use (param_type > PARAM_TYPE_INVALID) to check for invalid parameters, + so for now we will use a switch. This should also make GCC warn us when new types are added. */ + switch (param_type) { + + case PARAM_TYPE_UINT8: + case PARAM_TYPE_UINT16: + case PARAM_TYPE_UINT32: + case PARAM_TYPE_UINT64: + case PARAM_TYPE_INT8: + case PARAM_TYPE_INT16: + case PARAM_TYPE_INT32: + case PARAM_TYPE_INT64: + case PARAM_TYPE_XINT8: + case PARAM_TYPE_XINT16: + case PARAM_TYPE_XINT32: + case PARAM_TYPE_XINT64: + case PARAM_TYPE_FLOAT: + case PARAM_TYPE_DOUBLE: + case PARAM_TYPE_STRING: + case PARAM_TYPE_DATA: + case PARAM_TYPE_INVALID: // TODO Kevin: Is PARAM_TYPE_INVALID a valid type? + break; + + default: + PyErr_SetString(PyExc_InvalidParameterTypeError, "An invalid parameter type was specified during creation of a new parameter"); + return NULL; + } + + if (param_list_find_id(0, id) != NULL) { + /* Run away as quickly as possible if this ID is already in use, we would otherwise get a segfault, which is driving me insane. */ + PyErr_Format(PyExc_ValueError, "Parameter with id %d already exists", id); + return NULL; + } + + if (param_list_find_name(0, name)) { + /* While it is perhaps technically acceptable, it's probably best if we don't allow duplicate names either. */ + PyErr_Format(PyExc_ValueError, "Parameter with name \"%s\" already exists", name); + return NULL; + } + + if (!is_valid_callback(callback, true)) { + return NULL; // Exception message set by is_valid_callback(); + } + + param_t * new_param = param_list_create_remote(id, node, param_type, mask, array_size, name, unit, docstr, -1); + if (new_param == NULL) { + return (DynamicParameterObject*)PyErr_NoMemory(); + } + + DynamicParameterObject * self = (DynamicParameterObject *)_pycsh_Parameter_from_param(type, new_param, callback, host, timeout, retries, paramver); + if (self == NULL) { + /* This is likely a memory allocation error, in which case we expect .tp_alloc() to have raised an exception. */ + return NULL; + } + + /* NULL callback becomes None on a ParameterObject instance */ + if (callback == NULL) + callback = Py_None; + + if (Parameter_set_callback(self, (PyObject *)callback, NULL) == -1) { + Py_DECREF(self); + return NULL; + } + + ((ParameterObject*)self)->param->callback = Parameter_callback; + + return self; +} + +static PyObject * DynamicParameter_new(PyTypeObject *type, PyObject * args, PyObject * kwds) { + + uint16_t id; + char * name; + param_type_e param_type; + PyObject * mask_obj; + char * unit = ""; + char * docstr = ""; + int array_size = 0; + PyObject * callback = NULL; + uint16_t node = 0; + int host = INT_MIN; + // TODO Kevin: What are these 2 doing here? + int timeout = pycsh_dfl_timeout; + int retries = 0; + int paramver = 2; + + static char *kwlist[] = {"id", "name", "type", "mask", "unit", "docstr", "array_size", "callback", "node", "host", "timeout", "retries", "paramver", NULL}; + + if (!PyArg_ParseTupleAndKeywords(args, kwds, "HsiO|zziOHiiii", kwlist, &id, &name, ¶m_type, &mask_obj, &unit, &docstr, &array_size, &callback, &node, &host, &timeout, &retries, ¶mver)) + return NULL; // TypeError is thrown + + uint32_t mask; + if (pycsh_parse_param_mask(mask_obj, &mask) != 0) { + return NULL; // Exception message set by pycsh_parse_param_mask() + } + + if (array_size < 1) + array_size = 1; + + DynamicParameterObject * python_param = Parameter_create_new(type, id, node, param_type, mask, name, unit, docstr, array_size, callback, host, timeout, retries, paramver); + if (python_param == NULL) { + // Assume exception message to be set by Parameter_create_new() + /* physaddr should be freed in dealloc() */ + return NULL; + } + + /* return should steal the reference created by Parameter_create_new() */ + return (PyObject *)python_param; +} + + +static int _DynamicParameterObject_list_forget(DynamicParameterObject *self) { + assert(self); + param_list_remove_specific(self->parameter_object.param, pycsh_dfl_verbose, 0); + Py_DECREF(self); // Parameter list no longer holds a reference to self. + return 0; +} + + +/* Pulls all Parameters in the list as a single request. */ +static PyObject * DynamicParameter_list_add(DynamicParameterObject *self, PyObject *args, PyObject *kwds) { + + bool override = false; + + static char *kwlist[] = {"override", NULL}; + + if (!PyArg_ParseTupleAndKeywords(args, kwds, "|p", kwlist, &override)) + return NULL; // TypeError is thrown + + param_t * self_param = self->parameter_object.param; + param_t * existing = param_list_find_id(*self_param->node, self_param->id); + + assert(self_param); + if (existing == self_param) { + return Py_NewRef(self); + } + + if (existing) { + if (!override) { + PyErr_Format(PyExc_ValueError, "Parameter with ID %d on node %d already exists in the list (with name %s)", existing->id, existing->node, existing->name); + return NULL; + } + + ParameterObject * existing_python = Parameter_wraps_param(existing); + + if (existing_python) { + assert(PyObject_IsInstance((PyObject*)existing_python, (PyObject*)&DynamicParameterType)); + /* TODO Kevin: If existing is a DynamicParameter, we should handle reference counting when removing it from the list. */ + _DynamicParameterObject_list_forget((DynamicParameterObject*)existing_python); + } else { + /* We can't really know if the existing parameter should be destroyed, but doing it incorrectly */ + param_list_remove_specific(existing, pycsh_dfl_verbose, 1); + } + } + + int res = param_list_add(self->parameter_object.param); + assert(res == 0); + Py_INCREF(self); // Parameter list now holds a reference to self. + + Py_RETURN_NONE; +} + +static PyObject * DynamicParameter_list_forget(PyObject * self, PyObject * args) { + if (_DynamicParameterObject_list_forget((DynamicParameterObject*)self)) { + Py_RETURN_FALSE; + } else { + Py_RETURN_TRUE; + } +} + +static PyMethodDef DynamicParameter_methods[] = { + {"list_add", (PyCFunction)DynamicParameter_list_add, METH_VARARGS | METH_KEYWORDS, + PyDoc_STR("Adds `self` to the global parameter list")}, + {"list_forget", (PyCFunction)DynamicParameter_list_forget, METH_NOARGS, + PyDoc_STR("Removes `self` from the global parameter list")}, + {NULL, NULL, 0, NULL} +}; + +static PyObject * Parameter_get_callback(DynamicParameterObject *self, void *closure) { + return Py_NewRef(self->callback); +} + +static PyGetSetDef DynamicParameter_getsetters[] = { + //{"keep_alive", (getter)Parameter_get_keep_alive, (setter)Parameter_set_keep_alive, + // "Whether the Parameter should remain in the parameter list, when all Python references are lost. This makes it possible to recover the Parameter instance through list()", NULL}, + {"callback", (getter)Parameter_get_callback, (setter)Parameter_set_callback, + "callback of the parameter", NULL}, + {NULL, NULL, NULL, NULL} /* Sentinel */ +}; + +PyTypeObject DynamicParameterType = { + PyVarObject_HEAD_INIT(NULL, 0) + .tp_name = "pycsh.DynamicParameter", + .tp_doc = "Parameter created in Python.", + .tp_basicsize = sizeof(DynamicParameterObject), + .tp_itemsize = 0, + .tp_flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, + .tp_new = DynamicParameter_new, + .tp_dealloc = (destructor)DynamicParameter_dealloc, + .tp_getset = DynamicParameter_getsetters, + // .tp_str = (reprfunc)Parameter_str, + // .tp_richcompare = (richcmpfunc)Parameter_richcompare, + .tp_base = &ParameterType, + .tp_methods = DynamicParameter_methods, +}; diff --git a/lib/pycsh_core/src/parameter/dynamicparameter.h b/lib/pycsh_core/src/parameter/dynamicparameter.h new file mode 100644 index 0000000..0706134 --- /dev/null +++ b/lib/pycsh_core/src/parameter/dynamicparameter.h @@ -0,0 +1,45 @@ +/* + * dynamicparameter.h + * + * Contains the DynamicParameter Parameter subclass. + * + */ + +#pragma once + +#define PY_SSIZE_T_CLEAN +#include + +#include +#include + +#include "parameter.h" + +extern PyObject * PyExc_ParamCallbackError; +extern PyObject * PyExc_InvalidParameterTypeError; + +extern PyDictObject * param_callback_dict; + +typedef struct { + ParameterObject parameter_object; + PyObject *callback; +} DynamicParameterObject; + +extern PyTypeObject DynamicParameterType; + +void Parameter_callback(param_t * param, int offset); + +DynamicParameterObject * Parameter_create_new(PyTypeObject *type, uint16_t id, uint16_t node, param_type_e param_type, uint32_t mask, char * name, char * unit, char * docstr, int array_size, const PyObject * callback, int host, int timeout, int retries, int paramver); + +// Source: https://chat.openai.com +/** + * @brief Check that the callback accepts exactly one Parameter and one integer, + * as specified by "void (*callback)(struct param_s * param, int offset)" + * + * Currently also checks type-hints (if specified). + * + * @param callback function to check + * @param raise_exc Whether to set exception message when returning false. + * @return true for success + */ +bool is_valid_callback(const PyObject *callback, bool raise_exc); diff --git a/lib/pycsh_core/src/parameter/pythongetsetparameter.c b/lib/pycsh_core/src/parameter/getsetparameter.c similarity index 94% rename from lib/pycsh_core/src/parameter/pythongetsetparameter.c rename to lib/pycsh_core/src/parameter/getsetparameter.c index bbf9a2b..7512392 100644 --- a/lib/pycsh_core/src/parameter/pythongetsetparameter.c +++ b/lib/pycsh_core/src/parameter/getsetparameter.c @@ -5,7 +5,7 @@ * */ -#include "pythongetsetparameter.h" +#include "getsetparameter.h" // It is recommended to always define PY_SSIZE_T_CLEAN before including Python.h #define PY_SSIZE_T_CLEAN @@ -21,7 +21,7 @@ #include "pycshconfig.h" -PythonGetSetParameterObject *python_wraps_vmem(const vmem_t * vmem); +GetSetParameterObject *python_wraps_vmem(const vmem_t * vmem); static PyObject *_pycsh_val_to_pyobject(param_type_e type, const void * value) { switch (type) { @@ -186,7 +186,7 @@ void Parameter_getter(vmem_t * vmem, uint64_t addr, void * dataout, uint32_t len PyGILState_STATE CLEANUP_GIL gstate = PyGILState_Ensure(); assert(!PyErr_Occurred()); // Callback may raise an exception. But we don't want to override an existing one. - PythonGetSetParameterObject *python_param = python_wraps_vmem(vmem); + GetSetParameterObject *python_param = python_wraps_vmem(vmem); /* This param_t uses the Python Parameter callback, but doesn't actually point to a Parameter. Perhaps it was deleted? Or perhaps it was never set correctly. */ @@ -237,7 +237,7 @@ void Parameter_setter(vmem_t * vmem, uint64_t addr, const void * datain, uint32_ PyGILState_STATE CLEANUP_GIL gstate = PyGILState_Ensure(); assert(!PyErr_Occurred()); // Callback may raise an exception. But we don't want to override an existing one. - PythonGetSetParameterObject *python_param = python_wraps_vmem(vmem); + GetSetParameterObject *python_param = python_wraps_vmem(vmem); /* This param_t uses the Python Parameter callback, but doesn't actually point to a Parameter. Perhaps it was deleted? Or perhaps it was never set correctly. */ @@ -279,16 +279,16 @@ void Parameter_setter(vmem_t * vmem, uint64_t addr, const void * datain, uint32_ } /** - * @brief Check if this vmem is wrapped by a PythonGetSetParameterObject. + * @brief Check if this vmem is wrapped by a GetSetParameterObject. * * @return borrowed reference to the wrapping PythonSlashCommandObject if wrapped, otherwise NULL. */ -PythonGetSetParameterObject *python_wraps_vmem(const vmem_t * vmem) { +GetSetParameterObject *python_wraps_vmem(const vmem_t * vmem) { if (vmem == NULL || (vmem->read != Parameter_getter && vmem->write != Parameter_setter)) return NULL; // This slash command is not wrapped by PythonSlashCommandObject // TODO Kevin: What are the consequences of allowing only getter and or setter? // assert(vmem->write == Parameter_setter); // It should not be possible to have the correct internal .read(), but the incorrect internal .write() - return (PythonGetSetParameterObject *)((char *)vmem - offsetof(PythonGetSetParameterObject, vmem_heap)); + return (GetSetParameterObject *)((char *)vmem - offsetof(GetSetParameterObject, vmem_heap)); } // Source: https://chat.openai.com @@ -448,7 +448,7 @@ static bool is_valid_setter(const PyObject *setter, bool raise_exc) { return true; } -static void PythonGetSetParameter_dealloc(PythonGetSetParameterObject *self) { +static void PythonGetSetParameter_dealloc(GetSetParameterObject *self) { if (self->getter_func != NULL && self->getter_func != Py_None) { Py_XDECREF(self->getter_func); @@ -460,15 +460,15 @@ static void PythonGetSetParameter_dealloc(PythonGetSetParameterObject *self) { self->setter_func = NULL; } - PyTypeObject *baseclass = pycsh_get_base_dealloc_class(&PythonGetSetParameterType); + PyTypeObject *baseclass = pycsh_get_base_dealloc_class(&GetSetParameterType); baseclass->tp_dealloc((PyObject*)self); } -static PyObject * Parameter_get_getter(PythonGetSetParameterObject *self, void *closure) { +static PyObject * Parameter_get_getter(GetSetParameterObject *self, void *closure) { return Py_NewRef(self->getter_func); } -int Parameter_set_getter(PythonGetSetParameterObject *self, PyObject *value, void *closure) { +int Parameter_set_getter(GetSetParameterObject *self, PyObject *value, void *closure) { if (value == NULL) { PyErr_SetString(PyExc_TypeError, "Cannot delete the getter attribute"); @@ -512,11 +512,11 @@ int Parameter_set_getter(PythonGetSetParameterObject *self, PyObject *value, voi return 0; } -static PyObject * Parameter_get_setter(PythonGetSetParameterObject *self, void *closure) { +static PyObject * Parameter_get_setter(GetSetParameterObject *self, void *closure) { return Py_NewRef(self->setter_func); } -int Parameter_set_setter(PythonGetSetParameterObject *self, PyObject *value, void *closure) { +int Parameter_set_setter(GetSetParameterObject *self, PyObject *value, void *closure) { if (value == NULL) { PyErr_SetString(PyExc_TypeError, "Cannot delete the setter attribute"); @@ -608,7 +608,7 @@ static PyObject * PythonGetSetParameter_new(PyTypeObject *type, PyObject * args, array_size = 1; // TODO Kevin: Call super with correct *args and **kwargs, instead of reimplementing PythonParameter.__new__() - PythonGetSetParameterObject * self = (PythonGetSetParameterObject*)Parameter_create_new(type, id, param_type, mask, name, unit, docstr, array_size, callback, host, timeout, retries, paramver); + GetSetParameterObject * self = (GetSetParameterObject*)Parameter_create_new(type, id, 0, param_type, mask, name, unit, docstr, array_size, callback, host, timeout, retries, paramver); if (self == NULL) { // Assume exception message to be set by Parameter_create_new() /* physaddr should be freed in dealloc() */ @@ -652,11 +652,11 @@ static PyGetSetDef PythonParameter_getsetters[] = { {NULL, NULL, NULL, NULL} /* Sentinel */ }; -PyTypeObject PythonGetSetParameterType = { +PyTypeObject GetSetParameterType = { PyVarObject_HEAD_INIT(NULL, 0) .tp_name = "pycsh.PythonGetSetParameter", .tp_doc = "Parameter, with getter and or setter, created in Python.", - .tp_basicsize = sizeof(PythonGetSetParameterObject), + .tp_basicsize = sizeof(GetSetParameterObject), .tp_itemsize = 0, .tp_flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, .tp_new = PythonGetSetParameter_new, @@ -664,5 +664,5 @@ PyTypeObject PythonGetSetParameterType = { .tp_getset = PythonParameter_getsetters, // .tp_str = (reprfunc)Parameter_str, // .tp_richcompare = (richcmpfunc)Parameter_richcompare, - .tp_base = &PythonParameterType, + .tp_base = &DynamicParameterType, }; diff --git a/lib/pycsh_core/src/parameter/pythongetsetparameter.h b/lib/pycsh_core/src/parameter/getsetparameter.h similarity index 73% rename from lib/pycsh_core/src/parameter/pythongetsetparameter.h rename to lib/pycsh_core/src/parameter/getsetparameter.h index 13304d8..c8f0bae 100644 --- a/lib/pycsh_core/src/parameter/pythongetsetparameter.h +++ b/lib/pycsh_core/src/parameter/getsetparameter.h @@ -20,22 +20,22 @@ //extern PyObject * PyExc_ParamSetterError; typedef struct { - PythonParameterObject parameter_object; + DynamicParameterObject parameter_object; PyObject *getter_func; PyObject *setter_func; /* Every GetSetParameter instance allocates its own vmem. This vmem is passed to its read/write, - which allows us to work our way back to the PythonGetSetParameterObject, + which allows us to work our way back to the GetSetParameterObject, based on knowing the offset of the vmem field. - The PythonGetSetParameterObject is needed to call the Python getter/setter funcs. */ + The GetSetParameterObject is needed to call the Python getter/setter funcs. */ /* NOTE: PythonParameter (our baseclass) uses param_list_create_remote() to create its ->param, which both allocates and assign a vmem to ->param->vmem. This vmem will be overridden by the one you see below, but it will still be freed by param_list_remove_specific(). It would be nice to reuse it, - but it probably can't help us find our PythonGetSetParameterObject */ + but it probably can't help us find our GetSetParameterObject */ vmem_t vmem_heap; -} PythonGetSetParameterObject; +} GetSetParameterObject; -extern PyTypeObject PythonGetSetParameterType; +extern PyTypeObject GetSetParameterType; diff --git a/lib/pycsh_core/src/parameter/parameter.c b/lib/pycsh_core/src/parameter/parameter.c index 1c73165..53a557b 100644 --- a/lib/pycsh_core/src/parameter/parameter.c +++ b/lib/pycsh_core/src/parameter/parameter.c @@ -98,7 +98,7 @@ static PyObject * Parameter_get_id(ParameterObject *self, void *closure) { } static PyObject * Parameter_get_node(ParameterObject *self, void *closure) { - return Py_BuildValue("H", self->param->node); + return Py_BuildValue("H", *self->param->node); } static PyObject * Parameter_get_storage_type(ParameterObject *self, void *closure) { @@ -327,6 +327,72 @@ static void Parameter_dealloc(ParameterObject *self) { baseclass->tp_dealloc((PyObject*)self); } + +static PyObject * ParameterArray_GetItem(ParameterObject *self, PyObject *item) { + + if (!PyLong_Check(item)) { + PyErr_SetString(PyExc_TypeError, "Index must be an integer."); + return NULL; + } + + // _PyLong_AsInt is dependant on the Py_LIMITED_API macro, hence the underscore prefix. + int index = _PyLong_AsInt(item); + + // We can't use the fact that _PyLong_AsInt() returns -1 for error, + // because it may be our desired index. So we check for an exception instead. + if (PyErr_Occurred()) + return NULL; // 'Reraise' the current exception. + + /* TODO Kevin: How should we handle remote/cached when using indexed access? */ + return _pycsh_util_get_single(self->param, index, 1, self->host, self->timeout, self->retries, self->paramver, -1); +} + +static int ParameterArray_SetItem(ParameterObject *self, PyObject* item, PyObject* value) { + + if (value == NULL) { + PyErr_SetString(PyExc_TypeError, "Cannot delete parameter array indexes."); + return -1; + } + + if (!PyLong_Check(item)) { + PyErr_SetString(PyExc_TypeError, "Index must be an integer."); + return -2; + } + + // _PyLong_AsInt is dependant on the Py_LIMITED_API macro, hence the underscore prefix. + int index = _PyLong_AsInt(item); + + // We can't use the fact that _PyLong_AsInt() returns -1 for error, + // because it may be our desired index. So we check for an exception instead. + if (PyErr_Occurred()) + return -3; // 'Reraise' the current exception. + + // _pycsh_util_set_single() uses negative numbers for exceptions, + // so we just return its return value. +#if 0 /* TODO Kevin: When should we use queues with the new cmd system? */ + param_queue_t *usequeue = autosend ? NULL : ¶m_queue_set; +#endif + return _pycsh_util_set_single(self->param, value, index, self->host, self->timeout, self->retries, self->paramver, 1, pycsh_dfl_verbose); +} + +static Py_ssize_t ParameterArray_length(ParameterObject *self) { + #if 0 + // We currently raise an exception when getting len() of non-array type parameters. + // This stops PyCharm (Perhaps other IDE's) from showing their length as 0. ¯\_(ツ)_/¯ + if (self->param->array_size <= 1) { + PyErr_SetString(PyExc_AttributeError, "Non-array type parameter is not subscriptable"); + return -1; + } + #endif + return self->param->array_size; +} + +static PyMappingMethods ParameterArray_as_mapping = { + (lenfunc)ParameterArray_length, + (binaryfunc)ParameterArray_GetItem, + (objobjargproc)ParameterArray_SetItem +}; + /* The Python binding 'Parameter' class exposes most of its attributes through getters, as only its 'value', 'host' and 'node' are mutable, and even those are through setters. @@ -383,6 +449,7 @@ PyTypeObject ParameterType = { .tp_new = Parameter_new, .tp_dealloc = (destructor)Parameter_dealloc, .tp_getset = Parameter_getsetters, + .tp_as_mapping = &ParameterArray_as_mapping, // .tp_members = Parameter_members, // .tp_methods = Parameter_methods, .tp_str = (reprfunc)Parameter_str, diff --git a/lib/pycsh_core/src/parameter/parameterarray.c b/lib/pycsh_core/src/parameter/parameterarray.c deleted file mode 100644 index 3599ea9..0000000 --- a/lib/pycsh_core/src/parameter/parameterarray.c +++ /dev/null @@ -1,94 +0,0 @@ -/* - * parameterarray.c - * - * Contains the ParameterArray subclass. - * - * Created on: Apr 28, 2022 - * Author: Kevin Wallentin Carlsen - */ - -#include "parameterarray.h" - -#include "../pycsh.h" -#include "../utils.h" - - -static PyObject * ParameterArray_GetItem(ParameterObject *self, PyObject *item) { - - if (!PyLong_Check(item)) { - PyErr_SetString(PyExc_TypeError, "Index must be an integer."); - return NULL; - } - - // _PyLong_AsInt is dependant on the Py_LIMITED_API macro, hence the underscore prefix. - int index = _PyLong_AsInt(item); - - // We can't use the fact that _PyLong_AsInt() returns -1 for error, - // because it may be our desired index. So we check for an exception instead. - if (PyErr_Occurred()) - return NULL; // 'Reraise' the current exception. - - /* TODO Kevin: How should we handle remote/cached when using indexed access? */ - return _pycsh_util_get_single(self->param, index, 1, self->host, self->timeout, self->retries, self->paramver, -1); -} - -static int ParameterArray_SetItem(ParameterObject *self, PyObject* item, PyObject* value) { - - if (value == NULL) { - PyErr_SetString(PyExc_TypeError, "Cannot delete parameter array indexes."); - return -1; - } - - if (!PyLong_Check(item)) { - PyErr_SetString(PyExc_TypeError, "Index must be an integer."); - return -2; - } - - // _PyLong_AsInt is dependant on the Py_LIMITED_API macro, hence the underscore prefix. - int index = _PyLong_AsInt(item); - - // We can't use the fact that _PyLong_AsInt() returns -1 for error, - // because it may be our desired index. So we check for an exception instead. - if (PyErr_Occurred()) - return -3; // 'Reraise' the current exception. - - // _pycsh_util_set_single() uses negative numbers for exceptions, - // so we just return its return value. -#if 0 /* TODO Kevin: When should we use queues with the new cmd system? */ - param_queue_t *usequeue = autosend ? NULL : ¶m_queue_set; -#endif - return _pycsh_util_set_single(self->param, value, index, self->host, self->timeout, self->retries, self->paramver, 1, pycsh_dfl_verbose); -} - -static Py_ssize_t ParameterArray_length(ParameterObject *self) { - // We currently raise an exception when getting len() of non-array type parameters. - // This stops PyCharm (Perhaps other IDE's) from showing their length as 0. ¯\_(ツ)_/¯ - if (self->param->array_size <= 1) { - PyErr_SetString(PyExc_AttributeError, "Non-array type parameter is not subscriptable"); - return -1; - } - return self->param->array_size; -} - -static PyMappingMethods ParameterArray_as_mapping = { - (lenfunc)ParameterArray_length, - (binaryfunc)ParameterArray_GetItem, - (objobjargproc)ParameterArray_SetItem -}; - -PyTypeObject ParameterArrayType = { - PyVarObject_HEAD_INIT(NULL, 0) - .tp_name = "pycsh.ParameterArray", - .tp_doc = "Wrapper utility class for libparam array parameters.", - .tp_basicsize = sizeof(ParameterArrayObject), - .tp_itemsize = 0, - .tp_flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, - // .tp_new = Parameter_new, - // .tp_dealloc = (destructor)Parameter_dealloc, - // .tp_getset = Parameter_getsetters, - // .tp_str = (reprfunc)Parameter_str, - .tp_as_mapping = &ParameterArray_as_mapping, - // .tp_richcompare = (richcmpfunc)Parameter_richcompare, - .tp_base = &ParameterType, -}; - diff --git a/lib/pycsh_core/src/parameter/parameterarray.h b/lib/pycsh_core/src/parameter/parameterarray.h deleted file mode 100644 index 6bb9cf5..0000000 --- a/lib/pycsh_core/src/parameter/parameterarray.h +++ /dev/null @@ -1,21 +0,0 @@ -/* - * parameterarray.h - * - * Contains the ParameterArray subclass. - * - * Created on: Apr 28, 2022 - * Author: Kevin Wallentin Carlsen - */ - -#pragma once - -#define PY_SSIZE_T_CLEAN -#include - -#include "parameter.h" - -typedef struct { - ParameterObject parameter; -} ParameterArrayObject; - -extern PyTypeObject ParameterArrayType; \ No newline at end of file diff --git a/lib/pycsh_core/src/parameter/pythonarrayparameter.c b/lib/pycsh_core/src/parameter/pythonarrayparameter.c deleted file mode 100644 index df58ac8..0000000 --- a/lib/pycsh_core/src/parameter/pythonarrayparameter.c +++ /dev/null @@ -1,43 +0,0 @@ - -#include "pythonarrayparameter.h" - -#define PY_SSIZE_T_CLEAN -#include - -#include "../utils.h" -#include "pythonparameter.h" -#include "parameterarray.h" - -PyTypeObject * PythonArrayParameterType; - -// TODO Kevin: Ideally, PythonArrayParameterType.tp_dealloc (and its base classes) would call super, rather than providing a complete implementation themselves. - -/* TODO Kevin: Consider if this function could be called with __attribute__((constructor())), - we just need to ensure Python has been initialized beforehand. */ -PyTypeObject * create_pythonarrayparameter_type(void) { - - // Dynamically create the PythonArrayParameter type with multiple inheritance - PyObject *bases AUTO_DECREF = PyTuple_Pack(2, (PyObject *)&PythonParameterType, (PyObject *)&ParameterArrayType); - if (bases == NULL) { - return NULL; - } - - PyObject *dict AUTO_DECREF = PyDict_New(); - if (dict == NULL) { - return NULL; - } - - PyObject *name AUTO_DECREF = PyUnicode_FromString("PythonArrayParameter"); - if (name == NULL) { - return NULL; - } - - PythonArrayParameterType = (PyTypeObject*)PyObject_CallFunctionObjArgs((PyObject *)&PyType_Type, name, bases, dict, NULL); - if (PythonArrayParameterType == NULL) { - return NULL; - } - - // NOTE Kevin: Allocating PythonArrayParameterType on global scope here, and using memcpy(), seems to cause a segmentation fault when accessing the class. - // PythonArrayParameterType = *(PyTypeObject *)temp_PythonArrayParameterType; // Copy class to global, so it can be accessed like the other classes. - return PythonArrayParameterType; -} diff --git a/lib/pycsh_core/src/parameter/pythonarrayparameter.h b/lib/pycsh_core/src/parameter/pythonarrayparameter.h deleted file mode 100644 index 314cb38..0000000 --- a/lib/pycsh_core/src/parameter/pythonarrayparameter.h +++ /dev/null @@ -1,15 +0,0 @@ - -#define PY_SSIZE_T_CLEAN -#include - -#include "pythonparameter.h" - -typedef struct { - PythonParameterObject python_parameter; - // No need to include ParameterArrayObject explicitly - // since it only adds ParameterObject which is already in PythonParameterObject -} PythonArrayParameterObject; - -PyTypeObject * create_pythonarrayparameter_type(void); - -extern PyTypeObject * PythonArrayParameterType; diff --git a/lib/pycsh_core/src/parameter/pythongetsetarrayparameter.c b/lib/pycsh_core/src/parameter/pythongetsetarrayparameter.c deleted file mode 100644 index 4ba16d7..0000000 --- a/lib/pycsh_core/src/parameter/pythongetsetarrayparameter.c +++ /dev/null @@ -1,41 +0,0 @@ - -#include "pythongetsetarrayparameter.h" - -#define PY_SSIZE_T_CLEAN -#include - -#include "../utils.h" -#include "pythongetsetparameter.h" -#include "pythonarrayparameter.h" - -PyTypeObject * PythonGetSetArrayParameterType; - -// TODO Kevin: Ideally, PythonArrayParameterType.tp_dealloc (and its base classes) would call super, rather than providing a complete implementation themselves. - -/* TODO Kevin: Consider if this function could be called with __attribute__((constructor())), - we just need to ensure Python has been initialized beforehand. */ -PyTypeObject * create_pythongetsetarrayparameter_type(void) { - - // Dynamically create the PythonArrayParameter type with multiple inheritance - PyObject *bases AUTO_DECREF = PyTuple_Pack(2, (PyObject *)&PythonGetSetParameterType, (PyObject *)PythonArrayParameterType); - if (bases == NULL) { - return NULL; - } - - PyObject *dict AUTO_DECREF = PyDict_New(); - if (dict == NULL) { - return NULL; - } - - PyObject *name AUTO_DECREF = PyUnicode_FromString("PythonGetSetArrayParameter"); - if (name == NULL) { - return NULL; - } - - PythonGetSetArrayParameterType = (PyTypeObject*)PyObject_CallFunctionObjArgs((PyObject *)&PyType_Type, name, bases, dict, NULL); - if (PythonGetSetArrayParameterType == NULL) { - return NULL; - } - - return PythonGetSetArrayParameterType; -} diff --git a/lib/pycsh_core/src/parameter/pythongetsetarrayparameter.h b/lib/pycsh_core/src/parameter/pythongetsetarrayparameter.h deleted file mode 100644 index 7153a02..0000000 --- a/lib/pycsh_core/src/parameter/pythongetsetarrayparameter.h +++ /dev/null @@ -1,15 +0,0 @@ - -#define PY_SSIZE_T_CLEAN -#include - -#include "pythongetsetparameter.h" - -typedef struct { - PythonGetSetParameterObject python_getset_parameter; - // No need to include ParameterArrayObject explicitly - // since it only adds ParameterObject which is already in PythonParameterObject -} PythonGetSetArrayParameterObject; - -PyTypeObject * create_pythongetsetarrayparameter_type(void); - -extern PyTypeObject * PythonGetSetArrayParameterType; diff --git a/lib/pycsh_core/src/parameter/pythonparameter.c b/lib/pycsh_core/src/parameter/pythonparameter.c index c49bedf..9078364 100644 --- a/lib/pycsh_core/src/parameter/pythonparameter.c +++ b/lib/pycsh_core/src/parameter/pythonparameter.c @@ -17,386 +17,8 @@ #include "../pycsh.h" #include "../utils.h" +#include "dynamicparameter.h" -// Instantiated in our PyMODINIT_FUNC -PyObject * PyExc_ParamCallbackError; -PyObject * PyExc_InvalidParameterTypeError; - -/** - * @brief Shared callback for all param_t's wrapped by a Parameter instance. - */ -void Parameter_callback(param_t * param, int offset) { - PyGILState_STATE CLEANUP_GIL gstate = PyGILState_Ensure(); - assert(Parameter_wraps_param(param)); - assert(!PyErr_Occurred()); // Callback may raise an exception. But we don't want to override an existing one. - - PyObject *key AUTO_DECREF = PyLong_FromVoidPtr(param); - PythonParameterObject *python_param = (PythonParameterObject*)PyDict_GetItem((PyObject*)param_callback_dict, key); - - /* This param_t uses the Python Parameter callback, but doesn't actually point to a Parameter. - Perhaps it was deleted? Or perhaps it was never set correctly. */ - if (python_param == NULL) { - assert(false); // TODO Kevin: Is this situation worthy of an assert(), or should we just ignore it? - return; - } - - // PythonParameterObject *python_param = (PythonParameterObject *)((char *)param - offsetof(PythonParameterObject, parameter_object.param)); - PyObject *python_callback = python_param->callback; - - /* This Parameter has no callback */ - /* Python_callback should not be NULL here when Parameter_wraps_param(), but we will allow it for now... */ - if (python_callback == NULL || python_callback == Py_None) { - return; - } - - assert(PyCallable_Check(python_callback)); - /* Create the arguments. */ - PyObject *pyoffset AUTO_DECREF = Py_BuildValue("i", offset); - PyObject * args AUTO_DECREF = PyTuple_Pack(2, python_param, pyoffset); - /* Call the user Python callback */ - PyObject *value AUTO_DECREF = PyObject_CallObject(python_callback, args); - - if (PyErr_Occurred()) { - /* It may not be clear to the user, that the exception came from the callback, - we therefore chain unto the existing exception, for better clarity. */ - /* _PyErr_FormatFromCause() seems simpler than PyException_SetCause() and PyException_SetContext() */ - // TODO Kevin: It seems exceptions raised in the CSP thread are ignored. - _PyErr_FormatFromCause(PyExc_ParamCallbackError, "Error calling Python callback"); - #if PYCSH_HAVE_APM // TODO Kevin: This is pretty ugly, but we can't let the error propagate when building for APM, as there is no one but us to catch it. - /* It may not be clear to the user, that the exception came from the callback, - we therefore chain unto the existing exception, for better clarity. */ - /* _PyErr_FormatFromCause() seems simpler than PyException_SetCause() and PyException_SetContext() */ - // TODO Kevin: It seems exceptions raised in the CSP thread are ignored. - //_PyErr_FormatFromCause(PyExc_ParamCallbackError, "Error calling Python callback"); - PyErr_Print(); - #endif - } -} - -// Source: https://chat.openai.com -/** - * @brief Check that the callback accepts exactly one Parameter and one integer, - * as specified by "void (*callback)(struct param_s * param, int offset)" - * - * Currently also checks type-hints (if specified). - * Additional optional arguments are also allowed, - * as these can be disregarded by the caller. - * - * @param callback function to check - * @param raise_exc Whether to set exception message when returning false. - * @return true for success - */ -bool is_valid_callback(const PyObject *callback, bool raise_exc) { - - /*We currently allow both NULL and Py_None, - as those are valid to have on PythonParameterObject */ - if (callback == NULL || callback == Py_None) - return true; - - // Suppress the incompatible pointer type warning when AUTO_DECREF is used on subclasses of PyObject* - #pragma GCC diagnostic push - #pragma GCC diagnostic ignored "-Wincompatible-pointer-types" - - // Get the __code__ attribute of the function, and check that it is a PyCodeObject - // TODO Kevin: Hopefully it's safe to assume that PyObject_GetAttrString() won't mutate callback - PyCodeObject *func_code AUTO_DECREF = (PyCodeObject*)PyObject_GetAttrString((PyObject*)callback, "__code__"); - if (!func_code || !PyCode_Check(func_code)) { - if (raise_exc) - PyErr_SetString(PyExc_TypeError, "Provided callback must be callable"); - return false; - } - - int accepted_pos_args = pycsh_get_num_accepted_pos_args(callback, raise_exc); - if (accepted_pos_args < 2) { - if (raise_exc) - PyErr_SetString(PyExc_TypeError, "Provided callback must accept at least 2 positional arguments"); - return false; - } - - // Check for too many required arguments - int num_non_default_pos_args = pycsh_get_num_required_args(callback, raise_exc); - if (num_non_default_pos_args > 2) { - if (raise_exc) - PyErr_SetString(PyExc_TypeError, "Provided callback must not require more than 2 positional arguments"); - return false; - } - - // Get the __annotations__ attribute of the function - // TODO Kevin: Hopefully it's safe to assume that PyObject_GetAttrString() won't mutate callback - PyDictObject *func_annotations AUTO_DECREF = (PyDictObject *)PyObject_GetAttrString((PyObject*)callback, "__annotations__"); - - // Re-enable the warning - #pragma GCC diagnostic pop - - assert(PyDict_Check(func_annotations)); - if (!func_annotations) { - return true; // It's okay to not specify type-hints for the callback. - } - - // Get the parameters annotation - // PyCode_GetVarnames() exists and should be exposed, but it doesn't appear to be in any visible header. - PyObject *param_names AUTO_DECREF = PyObject_GetAttrString((PyObject*)func_code, "co_varnames");// PyCode_GetVarnames(func_code); - if (!param_names) { - return true; // Function parameters have not been annotated, this is probably okay. - } - - // Check if it's a tuple - if (!PyTuple_Check(param_names)) { - // TODO Kevin: Not sure what exception to set here. - if (raise_exc) - PyErr_Format(PyExc_TypeError, "param_names type \"%s\" %p", param_names->ob_type->tp_name, param_names); - return false; // Not sure when this fails, but it's probably bad if it does. - } - - PyObject *typing_module_name AUTO_DECREF = PyUnicode_FromString("typing"); - if (!typing_module_name) { - return false; - } - - PyObject *typing_module AUTO_DECREF = PyImport_Import(typing_module_name); - if (!typing_module) { - if (raise_exc) - PyErr_SetString(PyExc_ImportError, "Failed to import typing module"); - return false; - } - -#if 1 - PyObject *get_type_hints AUTO_DECREF = PyObject_GetAttrString(typing_module, "get_type_hints"); - if (!get_type_hints) { - if (raise_exc) - PyErr_SetString(PyExc_ImportError, "Failed to get 'get_type_hints()' function"); - return false; - } - assert(PyCallable_Check(get_type_hints)); - - - PyObject *type_hint_dict AUTO_DECREF = PyObject_CallFunctionObjArgs(get_type_hints, callback, NULL); - -#else - - PyObject *get_type_hints_name AUTO_DECREF = PyUnicode_FromString("get_type_hints"); - if (!get_type_hints_name) { - return false; - } - - PyObject *type_hint_dict AUTO_DECREF = PyObject_CallMethodObjArgs(typing_module, get_type_hints_name, callback, NULL); - if (!type_hint_dict) { - if (raise_exc) - PyErr_SetString(PyExc_ImportError, "Failed to get type hints of callback"); - return false; - } -#endif - - // TODO Kevin: Perhaps issue warnings for type-hint errors, instead of errors. - { // Checking first parameter type-hint - - // co_varnames may be too short for our index, if the signature has *args, but that's okay. - if (PyTuple_Size(param_names)-1 <= 0) { - return true; - } - - PyObject *param_name = PyTuple_GetItem(param_names, 0); - if (!param_name) { - if (raise_exc) - PyErr_SetString(PyExc_IndexError, "Could not get first parameter name"); - return false; - } - - PyObject *param_annotation = PyDict_GetItem(type_hint_dict, param_name); - if (param_annotation != NULL && param_annotation != Py_None) { - if (!PyType_Check(param_annotation)) { - if (raise_exc) - PyErr_Format(PyExc_TypeError, "First parameter annotation is %s, which is not a type", param_annotation->ob_type->tp_name); - return false; - } - if (!PyObject_IsSubclass(param_annotation, (PyObject *)&ParameterType)) { - if (raise_exc) - PyErr_Format(PyExc_TypeError, "First callback parameter should be type-hinted as Parameter (or subclass). (not %s)", param_annotation->ob_type->tp_name); - return false; - } - } - } - - { // Checking second parameter type-hint - - // co_varnames may be too short for our index, if the signature has *args, but that's okay. - if (PyTuple_Size(param_names)-1 <= 1) { - return true; - } - - PyObject *param_name = PyTuple_GetItem(param_names, 1); - if (!param_name) { - if (raise_exc) - PyErr_SetString(PyExc_IndexError, "Could not get first parameter name"); - return false; - } - - PyObject *param_annotation = PyDict_GetItem(type_hint_dict, param_name); - if (param_annotation != NULL && param_annotation != Py_None) { - if (!PyType_Check(param_annotation)) { - if (raise_exc) - PyErr_Format(PyExc_TypeError, "Second parameter annotation is %s, which is not a type", param_annotation->ob_type->tp_name); - return false; - } - if (!PyObject_IsSubclass(param_annotation, (PyObject *)&PyLong_Type)) { - if (raise_exc) - PyErr_Format(PyExc_TypeError, "Second callback parameter should be type-hinted as int offset. (not %s)", param_annotation->ob_type->tp_name); - return false; - } - } - } - - return true; -} - -static void PythonParameter_dealloc(PythonParameterObject *self) { - - if (self->callback != NULL && self->callback != Py_None) { - Py_XDECREF(self->callback); - self->callback = NULL; - } - - /* We defer deallocation to our Parameter baseclass, - as it must also handle deallocation of param_t's that have been "list forget"en anyway. */ - param_list_remove_specific(((ParameterObject*)self)->param, 0, 0); - - PyTypeObject *baseclass = pycsh_get_base_dealloc_class(&PythonParameterType); - baseclass->tp_dealloc((PyObject*)self); -} - -static int Parameter_set_callback(PythonParameterObject *self, PyObject *value, void *closure) { - - if (value == NULL) { - PyErr_SetString(PyExc_TypeError, "Cannot delete the callback attribute"); - return -1; - } - - if (!is_valid_callback(value, true)) { - return -1; - } - - if (value == self->callback) - return 0; // No work to do - - /* Changing the callback to None. */ - if (value == Py_None) { - if (self->callback != Py_None) { - /* We should not arrive here when the old value is Py_None, - but prevent Py_DECREF() on it at all cost. */ - Py_XDECREF(self->callback); - } - self->callback = Py_None; - return 0; - } - - /* We now know that 'value' is a new callable. */ - - /* When replacing a previous callable. */ - if (self->callback != Py_None) { - Py_XDECREF(self->callback); - } - - Py_INCREF(value); - self->callback = value; - - return 0; -} - -/* Internal API for creating a new PythonParameterObject. */ -__attribute__((malloc(PythonParameter_dealloc, 1))) -PythonParameterObject * Parameter_create_new(PyTypeObject *type, uint16_t id, param_type_e param_type, uint32_t mask, char * name, char * unit, char * docstr, int array_size, const PyObject * callback, int host, int timeout, int retries, int paramver) { - - /* Check for valid parameter type. param_list_create_remote() should always return NULL for errors, - but this allows us to raise a specific exception. */ - /* I'm not sure whether we can use (param_type > PARAM_TYPE_INVALID) to check for invalid parameters, - so for now we will use a switch. This should also make GCC warn us when new types are added. */ - switch (param_type) { - - case PARAM_TYPE_UINT8: - case PARAM_TYPE_UINT16: - case PARAM_TYPE_UINT32: - case PARAM_TYPE_UINT64: - case PARAM_TYPE_INT8: - case PARAM_TYPE_INT16: - case PARAM_TYPE_INT32: - case PARAM_TYPE_INT64: - case PARAM_TYPE_XINT8: - case PARAM_TYPE_XINT16: - case PARAM_TYPE_XINT32: - case PARAM_TYPE_XINT64: - case PARAM_TYPE_FLOAT: - case PARAM_TYPE_DOUBLE: - case PARAM_TYPE_STRING: - case PARAM_TYPE_DATA: - case PARAM_TYPE_INVALID: - break; - - default: - PyErr_SetString(PyExc_InvalidParameterTypeError, "An invalid parameter type was specified during creation of a new parameter"); - return NULL; - } - - if (param_list_find_id(0, id) != NULL) { - /* Run away as quickly as possible if this ID is already in use, we would otherwise get a segfault, which is driving me insane. */ - PyErr_Format(PyExc_ValueError, "Parameter with id %d already exists", id); - return NULL; - } - - if (param_list_find_name(0, name)) { - /* While it is perhaps technically acceptable, it's probably best if we don't allow duplicate names either. */ - PyErr_Format(PyExc_ValueError, "Parameter with name \"%s\" already exists", name); - return NULL; - } - - if (!is_valid_callback(callback, true)) { - return NULL; // Exception message set by is_valid_callback(); - } - - param_t * new_param = param_list_create_remote(id, 0, param_type, mask, array_size, name, unit, docstr, -1); - if (new_param == NULL) { - return (PythonParameterObject*)PyErr_NoMemory(); - } - - PythonParameterObject * self = (PythonParameterObject *)_pycsh_Parameter_from_param(type, new_param, callback, host, timeout, retries, paramver); - if (self == NULL) { - /* This is likely a memory allocation error, in which case we expect .tp_alloc() to have raised an exception. */ - return NULL; - } - - switch (param_list_add(new_param)) { - case 0: - break; // All good - case 1: { - // It shouldn't be possible to arrive here, except perhaps from race conditions. - PyErr_SetString(PyExc_KeyError, "Local parameter with the specifed ID already exists"); - Py_DECREF(self); - return NULL; - } - default: { - Py_DECREF(self); - assert(false); // list_dynamic=false ? - break; - } - } - - // ((ParameterObject *)self)->param->callback = Parameter_callback; // NOTE: This assignment is performed in _pycsh_Parameter_from_param() - self->keep_alive = 1; - Py_INCREF(self); // Parameter list holds a reference to the ParameterObject - /* NOTE: If .keep_alive defaults to False, then we should remove this Py_INCREF() */ - - /* NULL callback becomes None on a ParameterObject instance */ - if (callback == NULL) - callback = Py_None; - - if (Parameter_set_callback(self, (PyObject *)callback, NULL) == -1) { - Py_DECREF(self); - return NULL; - } - - ((ParameterObject*)self)->param->callback = Parameter_callback; - - return self; -} static PyObject * PythonParameter_new(PyTypeObject *type, PyObject * args, PyObject * kwds) { @@ -427,13 +49,45 @@ static PyObject * PythonParameter_new(PyTypeObject *type, PyObject * args, PyObj if (array_size < 1) array_size = 1; - PythonParameterObject * python_param = Parameter_create_new(type, id, param_type, mask, name, unit, docstr, array_size, callback, host, timeout, retries, paramver); +#if 0 + static bool first = true; + if (first) { + first = false; + fprintf(stderr, "`PythonParameter(...)` is deprecated, use `DynamicParameter(..., node=0, ...)` instead"); + } +#endif + + PyErr_WarnEx(PyExc_DeprecationWarning, "`PythonParameter(...)` is deprecated, use `DynamicParameter(..., node=0, ...)` instead", 1); + + assert(PyObject_IsSubclass((PyObject*)type, (PyObject*)&PythonParameterType)); + + PythonParameterObject * python_param = (PythonParameterObject*)Parameter_create_new(type, id, 0, param_type, mask, name, unit, docstr, array_size, callback, host, timeout, retries, paramver); if (python_param == NULL) { // Assume exception message to be set by Parameter_create_new() /* physaddr should be freed in dealloc() */ return NULL; } + switch (param_list_add(python_param->dyn_param.parameter_object.param)) { + case 0: + break; // All good + case 1: { + // It shouldn't be possible to arrive here, except perhaps from race conditions. + PyErr_SetString(PyExc_KeyError, "Local parameter with the specifed ID already exists"); + Py_DECREF(python_param); + return NULL; + } + default: { + Py_DECREF(python_param); + assert(false); // list_dynamic=false ? + break; + } + } + + python_param->keep_alive = 1; + Py_INCREF(python_param); // Parameter list holds a reference to the ParameterObject + /* NOTE: If .keep_alive defaults to False, then we should remove this Py_INCREF() */ + /* return should steal the reference created by Parameter_create_new() */ return (PyObject *)python_param; } @@ -465,15 +119,10 @@ static int Parameter_set_keep_alive(PythonParameterObject *self, PyObject *value return 0; } -static PyObject * Parameter_get_callback(PythonParameterObject *self, void *closure) { - return Py_NewRef(self->callback); -} static PyGetSetDef PythonParameter_getsetters[] = { {"keep_alive", (getter)Parameter_get_keep_alive, (setter)Parameter_set_keep_alive, "Whether the Parameter should remain in the parameter list, when all Python references are lost. This makes it possible to recover the Parameter instance through list()", NULL}, - {"callback", (getter)Parameter_get_callback, (setter)Parameter_set_callback, - "callback of the parameter", NULL}, {NULL, NULL, NULL, NULL} /* Sentinel */ }; @@ -485,9 +134,8 @@ PyTypeObject PythonParameterType = { .tp_itemsize = 0, .tp_flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, .tp_new = PythonParameter_new, - .tp_dealloc = (destructor)PythonParameter_dealloc, .tp_getset = PythonParameter_getsetters, // .tp_str = (reprfunc)Parameter_str, // .tp_richcompare = (richcmpfunc)Parameter_richcompare, - .tp_base = &ParameterType, + .tp_base = &DynamicParameterType, }; diff --git a/lib/pycsh_core/src/parameter/pythonparameter.h b/lib/pycsh_core/src/parameter/pythonparameter.h index f48bdb3..a7830cf 100644 --- a/lib/pycsh_core/src/parameter/pythonparameter.h +++ b/lib/pycsh_core/src/parameter/pythonparameter.h @@ -14,24 +14,15 @@ #include #include "parameter.h" - -extern PyObject * PyExc_ParamCallbackError; -extern PyObject * PyExc_InvalidParameterTypeError; - -extern PyDictObject * param_callback_dict; +#include "dynamicparameter.h" typedef struct { - ParameterObject parameter_object; - PyObject *callback; + DynamicParameterObject dyn_param; int keep_alive: 1; // For the sake of reference counting, keep_alive should only be changed by Parameter_setkeep_alive() } PythonParameterObject; extern PyTypeObject PythonParameterType; -void Parameter_callback(param_t * param, int offset); - -PythonParameterObject * Parameter_create_new(PyTypeObject *type, uint16_t id, param_type_e param_type, uint32_t mask, char * name, char * unit, char * docstr, int array_size, const PyObject * callback, int host, int timeout, int retries, int paramver); - // Source: https://chat.openai.com /** * @brief Check that the callback accepts exactly one Parameter and one integer, diff --git a/lib/pycsh_core/src/pycsh.c b/lib/pycsh_core/src/pycsh.c index e97e183..f5be79a 100644 --- a/lib/pycsh_core/src/pycsh.c +++ b/lib/pycsh_core/src/pycsh.c @@ -53,11 +53,9 @@ #include "utils.h" #include "parameter/parameter.h" -#include "parameter/parameterarray.h" #include "parameter/pythonparameter.h" -#include "parameter/pythonarrayparameter.h" -#include "parameter/pythongetsetparameter.h" -#include "parameter/pythongetsetarrayparameter.h" +#include "parameter/dynamicparameter.h" +#include "parameter/getsetparameter.h" #include "parameter/parameterlist.h" #include "csp_classes/ident.h" @@ -401,7 +399,7 @@ PyMODINIT_FUNC PyInit_pycsh(void) { return NULL; } - if (PyModule_AddType(pycsh, &ParameterArrayType) < 0) { + if (PyModule_AddType(pycsh, &DynamicParameterType) < 0) { return NULL; } @@ -409,25 +407,7 @@ PyMODINIT_FUNC PyInit_pycsh(void) { return NULL; } - /* PythonArrayParameterType must be created dynamically after - ParameterArrayType and PythonParameterType to support multiple inheritance. */ - if (create_pythonarrayparameter_type() == NULL) { - return NULL; - } - if (PyModule_AddType(pycsh, PythonArrayParameterType) < 0) { - return NULL; - } - - if (PyModule_AddType(pycsh, &PythonGetSetParameterType) < 0) { - return NULL; - } - - /* PythonArrayParameterType must be created dynamically after - ParameterArrayType and PythonParameterType to support multiple inheritance. */ - if (create_pythongetsetarrayparameter_type() == NULL) { - return NULL; - } - if (PyModule_AddType(pycsh, PythonGetSetArrayParameterType) < 0) { + if (PyModule_AddType(pycsh, &GetSetParameterType) < 0) { return NULL; } diff --git a/lib/pycsh_core/src/utils.c b/lib/pycsh_core/src/utils.c index b397ddb..13c8e44 100644 --- a/lib/pycsh_core/src/utils.c +++ b/lib/pycsh_core/src/utils.c @@ -18,10 +18,8 @@ #include "pycsh.h" #include "parameter/parameter.h" -#include "parameter/parameterarray.h" -#include "parameter/pythonparameter.h" +#include "parameter/dynamicparameter.h" #include "parameter/parameterlist.h" -#include "parameter/pythonarrayparameter.h" #undef NDEBUG #include @@ -384,51 +382,6 @@ ParameterObject * Parameter_wraps_param(param_t *param) { return python_param; } -static PyTypeObject * get_arrayparameter_subclass(PyTypeObject *type) { - - // Get the __subclasses__ method - PyObject *subclasses_method AUTO_DECREF = PyObject_GetAttrString((PyObject *)type, "__subclasses__"); - if (subclasses_method == NULL) { - return NULL; - } - - // NOTE: .__subclasses__() is not recursive, but this is currently not an issue with ParameterArray and PythonArrayParameter - - // Call the __subclasses__ method - PyObject *subclasses_list AUTO_DECREF = PyObject_CallObject(subclasses_method, NULL); - if (subclasses_list == NULL) { - return NULL; - } - - // Ensure the result is a list - if (!PyList_Check(subclasses_list)) { - PyErr_SetString(PyExc_TypeError, "__subclasses__ did not return a list"); - return NULL; - } - - // Iterate over the list of subclasses - Py_ssize_t num_subclasses = PyList_Size(subclasses_list); - for (Py_ssize_t i = 0; i < num_subclasses; i++) { - PyObject *subclass = PyList_GetItem(subclasses_list, i); // Borrowed reference - if (subclass == NULL) { - return NULL; - } - - int is_subclass = PyObject_IsSubclass(subclass, (PyObject*)&ParameterArrayType); - if (is_subclass < 0) { - return NULL; - } - - PyErr_Clear(); - if (is_subclass) { - return (PyTypeObject*)subclass; - } - } - - PyErr_Format(PyExc_TypeError, "Failed to find ArrayParameter variant of class %s", type->tp_name); - return NULL; -} - /* Create a Python Parameter object from a param_t pointer directly. */ PyObject * _pycsh_Parameter_from_param(PyTypeObject *type, param_t * param, const PyObject * callback, int host, int timeout, int retries, int paramver) { if (param == NULL) { @@ -441,19 +394,6 @@ PyObject * _pycsh_Parameter_from_param(PyTypeObject *type, param_t * param, cons return (PyObject*)Py_NewRef(existing_parameter); } - if (param->array_size <= 1 && type == &ParameterArrayType) { - PyErr_SetString(PyExc_TypeError, - "Attempted to create a ParameterArray instance, for a non array parameter."); - return NULL; - } else if (param->array_size > 1) { // If the parameter is an array. - type = get_arrayparameter_subclass(type); // We create a ParameterArray instance instead. - if (type == NULL) { - return NULL; - } - // If you listen really carefully here, you can hear OOP idealists, screaming in agony. - // On a more serious note, I'm amazed that this even works at all. - } - ParameterObject *self = (ParameterObject *)type->tp_alloc(type, 0); if (self == NULL) @@ -559,11 +499,7 @@ static void pycsh_param_transaction_callback_pull(csp_packet_t *response, int ve queue.client_timestamp = time_now.tv_sec; queue.last_timestamp = queue.client_timestamp; - /* Even though we have been provided a `param_t * param`, - we still call `param_queue_apply()` to support replies which unexpectedly contain multiple parameters. - Although we are SOL if those unexpected parameters are not in the list. - TODO Kevin: Make sure ParameterList accounts for this scenario. */ - param_queue_apply(&queue, 0, from); + bool list_apply = false; int atomic_write = 0; for (size_t i = 0; i < param_list->cnt; i++) { /* Apply value to provided param_t* if they aren't in the list. */ @@ -582,6 +518,7 @@ static void pycsh_param_transaction_callback_pull(csp_packet_t *response, int ve /* List parameters have already been applied by param_queue_apply(). */ param_t * list_param = param_list_find_id(node, id); if (list_param != NULL) { + list_apply = true; /* Print the local RAM copy of the remote parameter (which is not in the list) */ if (verbose) { param_print(param, -1, NULL, 0, verbose, 0); @@ -593,6 +530,7 @@ static void pycsh_param_transaction_callback_pull(csp_packet_t *response, int ve /* This is not our parameter, let's hope it has been applied by `param_queue_apply(...)` */ if (param->id != id) { + list_apply = true; /* We need to discard the data field, to get to next paramid */ mpack_discard(&reader); continue; @@ -614,6 +552,17 @@ static void pycsh_param_transaction_callback_pull(csp_packet_t *response, int ve } } + if (list_apply) { + /* Even though we have been provided a `param_t * param`, + we still call `param_queue_apply()` to support replies which unexpectedly contain multiple parameters. + Although we are SOL if those unexpected parameters are not in the list. + TODO Kevin: Make sure ParameterList accounts for this scenario. */ + /* We cannot yet handle atomic write for expected list-less parameters that have been added to the list, + as param_queue_apply() will also enter critical. */ + assert(!atomic_write); + param_queue_apply(&queue, 0, from); + } + if (atomic_write) { if (param_exit_critical) param_exit_critical(); @@ -649,6 +598,58 @@ static int pycsh_param_pull_single(param_t *param, int offset, int prio, int ver return param_transaction(packet, host, timeout, pycsh_param_transaction_callback_pull, verbose, version, ¶m_list); } +static int pycsh_param_push_single(param_t *param, int offset, int prio, void *value, int verbose, int host, int timeout, int version, bool ack_with_pull) { + + csp_packet_t * packet = csp_buffer_get(PARAM_SERVER_MTU); + if (packet == NULL) + return -1; + + packet->data[1] = 0; + param_transaction_callback_f cb = NULL; + + if (ack_with_pull) { + packet->data[1] = 1; + cb = pycsh_param_transaction_callback_pull; + } + + param_list_t param_list = { + .param_arr = ¶m, + .cnt = 1 + }; + + if(version == 2) { + packet->data[0] = PARAM_PUSH_REQUEST_V2; + } else { + packet->data[0] = PARAM_PUSH_REQUEST; + } + + param_queue_t queue; + param_queue_init(&queue, &packet->data[2], PARAM_SERVER_MTU - 2, 0, PARAM_QUEUE_TYPE_SET, version); + param_queue_add(&queue, param, offset, value); + + packet->length = queue.used + 2; + packet->id.pri = prio; + int result = param_transaction(packet, host, timeout, cb, verbose, version, ¶m_list); + + if (result < 0) { + return -1; + } + + /* If it was a remote parameter, set the value after the ack but not if ack with push which sets param timestamp */ + if (*param->node != 0 && value != NULL && *param->timestamp == 0) + { + if (offset < 0) { + for (int i = 0; i < param->array_size; i++) + param_set(param, i, value); + } else { + param_set(param, offset, value); + } + } + + return 0; +} + + /* Checks that the specified index is within bounds of the sequence index, raises IndexError if not. Supports Python backwards subscriptions, mutates the index to a positive value in such cases. */ static int _pycsh_util_index(int seqlen, int *index) { @@ -1011,7 +1012,7 @@ int _pycsh_util_set_single(param_t *param, PyObject *value, int offset, int host for (size_t i = 0; i < (retries > 0 ? retries : 1); i++) { int param_push_res; Py_BEGIN_ALLOW_THREADS; // Only allow threads for remote parameters, as local ones could have Python callbacks. - param_push_res = param_push_single(param, offset, 0, valuebuf, 1, dest, timeout, paramver, true); + param_push_res = pycsh_param_push_single(param, offset, 0, valuebuf, 1, dest, timeout, paramver, true); Py_END_ALLOW_THREADS; if (param_push_res < 0) if (i >= retries-1) { diff --git a/lib/pycsh_core/src/wrapper/param_list_py.c b/lib/pycsh_core/src/wrapper/param_list_py.c index d225b0e..5a7630a 100644 --- a/lib/pycsh_core/src/wrapper/param_list_py.c +++ b/lib/pycsh_core/src/wrapper/param_list_py.c @@ -14,7 +14,7 @@ #include "../pycsh.h" #include "../utils.h" #include "../parameter/parameter.h" -#include "../parameter/pythonparameter.h" +#include "../parameter/dynamicparameter.h" #include "param_list_py.h"