From e91913f43cc4f78e5322d8f2e292fa9cd831dd35 Mon Sep 17 00:00:00 2001 From: kivkiv12345 Date: Wed, 7 May 2025 19:40:37 +0200 Subject: [PATCH] DynamicParameter class ack_with_pull still seems a little funky before it has been added to the list. There's also still some edge cases if the server responds with multiple parameters, of which only some are in the list (and or atomic write) --- lib/pycsh_core/meson.build | 6 +- lib/pycsh_core/pycsh.pyi | 39 +- .../src/parameter/dynamicparameter.c | 515 ++++++++++++++++++ .../src/parameter/dynamicparameter.h | 45 ++ ...hongetsetparameter.c => getsetparameter.c} | 34 +- ...hongetsetparameter.h => getsetparameter.h} | 12 +- lib/pycsh_core/src/parameter/parameter.c | 69 ++- lib/pycsh_core/src/parameter/parameterarray.c | 94 ---- lib/pycsh_core/src/parameter/parameterarray.h | 21 - .../src/parameter/pythonarrayparameter.c | 43 -- .../src/parameter/pythonarrayparameter.h | 15 - .../parameter/pythongetsetarrayparameter.c | 41 -- .../parameter/pythongetsetarrayparameter.h | 15 - .../src/parameter/pythonparameter.c | 422 ++------------ .../src/parameter/pythonparameter.h | 13 +- lib/pycsh_core/src/pycsh.c | 28 +- lib/pycsh_core/src/utils.c | 135 ++--- lib/pycsh_core/src/wrapper/param_list_py.c | 2 +- 18 files changed, 793 insertions(+), 756 deletions(-) create mode 100644 lib/pycsh_core/src/parameter/dynamicparameter.c create mode 100644 lib/pycsh_core/src/parameter/dynamicparameter.h rename lib/pycsh_core/src/parameter/{pythongetsetparameter.c => getsetparameter.c} (94%) rename lib/pycsh_core/src/parameter/{pythongetsetparameter.h => getsetparameter.h} (73%) delete mode 100644 lib/pycsh_core/src/parameter/parameterarray.c delete mode 100644 lib/pycsh_core/src/parameter/parameterarray.h delete mode 100644 lib/pycsh_core/src/parameter/pythonarrayparameter.c delete mode 100644 lib/pycsh_core/src/parameter/pythonarrayparameter.h delete mode 100644 lib/pycsh_core/src/parameter/pythongetsetarrayparameter.c delete mode 100644 lib/pycsh_core/src/parameter/pythongetsetarrayparameter.h 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"