Skip to content
Open
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
40c9103
Build system for python-wrapper
Mar 14, 2025
7302be2
Added wrapper dependencies to Dockerfiles
Mar 14, 2025
3703c20
Python-Wrapper bindings
Mar 14, 2025
76ec913
Added Tests for the Python-Wrapper
Mar 14, 2025
24abdb7
node.cpp Bugfix
Mar 14, 2025
640e815
CMake Python-Wrapper test integration, build fix
Mar 24, 2025
862d498
Applied changes as per feedback.
Mar 24, 2025
c48be8f
Merge branch 'master' into python-wrapper
al3xa23 Apr 7, 2025
73ce98f
Fixed typo
Apr 8, 2025
168df21
Explicitly disallowed copy and assignment
Apr 8, 2025
e710de5
CMakeLists.txt fix
Apr 8, 2025
b1e9a2f
Removed unnecessary dockerfile dependencies
May 14, 2025
365f88b
Renamed wrapper -> binding, LLVM format
May 14, 2025
6806d24
Changes for pipeline test integration
May 14, 2025
c5026a2
Merge branch 'master' into python-wrapper
al3xa23 May 15, 2025
9216366
Binding test formatted with black
May 15, 2025
2d7628f
Format Python binding test with black-jupyter
May 22, 2025
07cffec
Align python-devel package for consistency
Jun 12, 2025
d9d12de
Moved clients/python-binding/ to python/binding/
Jul 17, 2025
a146ecb
python-binding unit test split, socket fixes
Jul 17, 2025
22714fe
unit tests: add test and enable test discovery
Sep 8, 2025
052f419
integration tests: added and enable test discovery
Sep 8, 2025
be2b770
python binding: add wrapper and binding tweaks
Sep 9, 2025
5b887a4
binding tests: add binding wrapper tests
Sep 9, 2025
279eb80
binding tests: CI fix and reformat
Sep 9, 2025
66e1f14
binding wrapper: CI/build fix, add binding stubs
Sep 10, 2025
f73fc67
binding stubs: add missing SPDX header
Sep 12, 2025
124a1f5
python binding: install and fix CI
Sep 12, 2025
f023f4d
Merge branch 'master' into python-wrapper
Sep 12, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .gitlab-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,8 @@ test:unit:
image: ${DOCKER_IMAGE_DEV}:${DOCKER_TAG}
script:
- cmake -S . -B build ${CMAKE_OPTS}
- cmake --build build ${CMAKE_BUILD_OPTS} --target run-unit-tests run-unit-tests-common
- export PYTHONPATH=$PYTHONPATH:${PWD}/build/clients/python-binding
- cmake --build build ${CMAKE_BUILD_OPTS} --target run-unit-tests run-unit-tests-common run-python-binding-tests
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

regarding:
#884 (comment)

I am not quite sure whether or not it is better to export the path of the binding .so as is done here or
to make use of something like this:

paths:
  -build/clients/python-binding

I am not sure whether or not this would even work.

needs:
- job: "build:source: [fedora]"
artifacts: true
Expand Down
2 changes: 1 addition & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
# SPDX-FileCopyrightText: 2014-2023 Institute for Automation of Complex Power Systems, RWTH Aachen University
# SPDX-License-Identifier: Apache-2.0

cmake_minimum_required(VERSION 3.14)
cmake_minimum_required(VERSION 3.15)

project(villas-node
VERSION 0.0.0
Expand Down
1 change: 1 addition & 0 deletions clients/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,5 @@
# SPDX-License-Identifier: Apache-2.0

add_subdirectory(opal-rt/rtlab-asyncip)
add_subdirectory(python-binding)
add_subdirectory(shmem)
36 changes: 36 additions & 0 deletions clients/python-binding/CMakeLists.txt
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we move this to python/binding?

The clients folder was created to collect external code which interacts with VILLASnode. But, I would consider the Python bindings to be a more integral part of VILLASnode as its actually executing code from libvillas here.

Furthermore, this would keep all the VILLASnode / Python-related code in on place (the python directory).

This brings also up another question. I've seen that you named the native Python extension villas_node. This does not seem to fit well in the naming schema of the existing Python packages for VILLASnode:

  • villas
    • villas.node
    • villas.controller
    • villas.dataprocessing

See also here: https://pypi.org/search/?q=villas-

Ideally, I would like to bundle your bindings into a villas.node.pybind package and also publish it to PyPi at some point.

That would allow for installing the bindings, including the native extension, via pip install villas-node`.

What do you think about it? Let me know if I can support you here.

Copy link
Collaborator

@KeVteL KeVteL Jun 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moving the folder to python/binding seems like a good idea to me.
I suppose that the name of the module should be changed to villas-node-binding or villas-node-pybind?

About the integration with CMake and packaging:

I initially tried integrating the python-binding into the CMake build system to generate a package/wheel and install it during the process.
This turned out to be quite messy from the install/build side of things - besides me failing to get it working.

But during all the trial and error some concerns came up, which I believe to be very relevant when considering pybind11, Python bindings in general and packaging it.

  • Python bindings are tightly coupled to the Python version used when compiling against the Python C API.
  • They also link - at least to my understanding - against a particular version of the library (in this case libvillas), which may vary depending on the system libraries during the build.

As a pip package can not control system libraries and dependencies - not that it ever should the case - this makes packaging the binding problematic to say the least.
Unfortunately you can not make pip install or build VILLASnode itself, which would solve some a lot of issues in this case, can you?

What is your take on this?

Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
# Python-binding CMakeLists.
#
# Author: Kevin Vu te Laar <[email protected]>
# SPDX-FileCopyrightText: 2014-2025 Institute for Automation of Complex Power Systems, RWTH Aachen University
# SPDX-License-Identifier: Apache-2.0

set(PYBIND11_FINDPYTHON ON)
find_package(pybind11 CONFIG)

if(pybind11_FOUND)
find_package(Python3 REQUIRED COMPONENTS Interpreter Development)

execute_process(
COMMAND "${Python3_EXECUTABLE}" -c "import sysconfig; print(sysconfig.get_path('stdlib') + '/lib-dynload')"
OUTPUT_VARIABLE PYTHON_LIB_DYNLOAD_DIR
OUTPUT_STRIP_TRAILING_WHITESPACE
)

message(STATUS "Found Python version: ${Python_VERSION}")
message(STATUS "Python major version: ${Python_VERSION_MAJOR}")
message(STATUS "Python minor version: ${Python_VERSION_MINOR}")
message(STATUS "Python .so install directory: ${PYTHON_LIB_DYNLOAD_DIR}")

pybind11_add_module(python-binding villas-python-binding.cpp)
set_target_properties(python-binding PROPERTIES OUTPUT_NAME villas_node)
target_link_libraries(python-binding PUBLIC villas)

install(
TARGETS python-binding
COMPONENT lib
RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR}
LIBRARY DESTINATION ${PYTHON_LIB_DYNLOAD_DIR}
)
else()
message(STATUS "pybind11 not found. Skipping Python wrapper build.")
endif()
226 changes: 226 additions & 0 deletions clients/python-binding/villas-python-binding.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,226 @@
/* Python-binding.
*
* Author: Kevin Vu te Laar <[email protected]>
* SPDX-FileCopyrightText: 2014-2025 Institute for Automation of Complex Power
* Systems, RWTH Aachen University SPDX-License-Identifier: Apache-2.0
*/

#include <jansson.h>
#include <pybind11/pybind11.h>
#include <uuid/uuid.h>

#include <villas/node.hpp>
#include <villas/sample.hpp>

extern "C" {
#include <villas/node.h>
}

namespace py = pybind11;

class Array {
public:
Array(unsigned int len) {
smps = new vsample *[len]();
this->len = len;
}
Array(const Array &) = delete;
Array &operator=(const Array &) = delete;

~Array() {
for (unsigned int i = 0; i < len; ++i) {
sample_decref(smps[i]);
smps[i] = nullptr;
}
delete[] smps;
}

vsample *&operator[](unsigned int idx) { return smps[idx]; }

vsample *&operator[](unsigned int idx) const { return smps[idx]; }

vsample **get_smps() { return smps; }

unsigned int size() const { return len; }

private:
vsample **smps;
unsigned int len;
};

/* pybind11 can not deal with (void **) as function input parameters,
* therefore we have to cast a simple (void *) pointer to the corresponding type
*
* wrapper bindings, sorted alphabetically
* @param villas_node Name of the module to be bound
* @param m Access variable for modifying the module code
*/
PYBIND11_MODULE(villas_node, m) {
m.def("memory_init", &memory_init);

m.def("node_check", [](void *n) -> int { return node_check((vnode *)n); });

m.def("node_destroy",
[](void *n) -> int { return node_destroy((vnode *)n); });

m.def(
"node_details",
[](void *n) -> const char * { return node_details((vnode *)n); },
py::return_value_policy::copy);

m.def("node_input_signals_max_cnt", [](void *n) -> unsigned {
return node_input_signals_max_cnt((vnode *)n);
});

m.def("node_is_enabled",
[](void *n) -> bool { return node_is_enabled((const vnode *)n); });

m.def("node_is_valid_name",
[](const char *name) -> bool { return node_is_valid_name(name); });

m.def(
"node_name",
[](void *n) -> const char * { return node_name((vnode *)n); },
py::return_value_policy::copy);

m.def(
"node_name_full",
[](void *n) -> const char * { return node_name_full((vnode *)n); },
py::return_value_policy::copy);

m.def(
"node_name_short",
[](void *n) -> const char * { return node_name_short((vnode *)n); },
py::return_value_policy::copy);

m.def("node_netem_fds", [](void *n, int fds[]) -> int {
return node_netem_fds((vnode *)n, fds);
});

m.def(
"node_new",
[](const char *id_str, const char *json_str) -> vnode * {
json_error_t err;
uuid_t id;

uuid_parse(id_str, id);
auto *json = json_loads(json_str, 0, &err);

void *it = json_object_iter(json);
json_t *inner = json_object_iter_value(it);

if (json_is_object(inner)) { // create node with name
return (vnode *)villas::node::NodeFactory::make(
json_object_iter_value(it), id, json_object_iter_key(it));
} else { // create node without name
char *capi_str = json_dumps(json, 0);
auto ret = node_new(id_str, capi_str);

free(capi_str);
return ret;
}
},
py::return_value_policy::reference);

m.def("node_output_signals_max_cnt", [](void *n) -> unsigned {
return node_output_signals_max_cnt((vnode *)n);
});

m.def("node_pause", [](void *n) -> int { return node_pause((vnode *)n); });

m.def("node_poll_fds", [](void *n, int fds[]) -> int {
return node_poll_fds((vnode *)n, fds);
});

m.def("node_prepare",
[](void *n) -> int { return node_prepare((vsample *)n); });

m.def("node_read", [](void *n, Array &a, unsigned cnt) -> int {
return node_read((vnode *)n, a.get_smps(), cnt);
});

m.def("node_restart",
[](void *n) -> int { return node_restart((vnode *)n); });

m.def("node_resume", [](void *n) -> int { return node_resume((vnode *)n); });

m.def("node_reverse",
[](void *n) -> int { return node_reverse((vnode *)n); });

m.def("node_start", [](void *n) -> int { return node_start((vnode *)n); });

m.def("node_stop", [](void *n) -> int { return node_stop((vnode *)n); });

m.def("node_to_json", [](void *n) -> py::str {
auto json = reinterpret_cast<villas::node::Node *>(n)->toJson();
char *json_str = json_dumps(json, 0);
auto py_str = py::str(json_str);

json_decref(json);
free(json_str);

return py_str;
});

m.def("node_to_json_str", [](void *n) -> py::str {
auto json = reinterpret_cast<villas::node::Node *>(n)->toJson();
char *json_str = json_dumps(json, 0);
auto py_str = py::str(json_str);

json_decref(json);
free(json_str);

return py_str;
});

m.def("node_write", [](void *n, Array &a, unsigned cnt) -> int {
return node_write((vnode *)n, a.get_smps(), cnt);
});

m.def(
"smps_array", [](unsigned int len) -> Array * { return new Array(len); },
py::return_value_policy::take_ownership);

m.def("sample_alloc",
[](unsigned int len) -> vsample * { return sample_alloc(len); });

// Decrease reference count and release memory if last reference was held.
m.def("sample_decref", [](void *smps) -> void {
auto smp = (vsample **)smps;
sample_decref(*smp);
});

m.def("sample_length",
[](void *smp) -> unsigned { return sample_length((vsample *)smp); });

m.def("sample_pack", &sample_pack, py::return_value_policy::reference);

m.def(
"sample_unpack",
[](void *smp, unsigned *seq, struct timespec *ts_origin,
struct timespec *ts_received, int *flags, unsigned *len,
double *values) -> void {
return sample_unpack((vsample *)smp, seq, ts_origin, ts_received, flags,
len, values);
},
py::return_value_policy::reference);

py::class_<Array>(m, "SamplesArray")
.def(py::init<unsigned int>(), py::arg("len"))
.def("__getitem__",
[](Array &a, unsigned int idx) {
if (idx >= a.size()) {
throw py::index_error("Index out of bounds");
}
return a[idx];
})
.def("__setitem__", [](Array &a, unsigned int idx, void *smp) {
if (idx >= a.size()) {
throw py::index_error("Index out of bounds");
}
if (a[idx]) {
sample_decref(a[idx]);
}
a[idx] = (vsample *)smp;
});
}
2 changes: 1 addition & 1 deletion lib/node.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -439,7 +439,7 @@ Node *NodeFactory::make(json_t *json, const uuid_t &id,
std::string type;
Node *n;

if (json_is_object(json))
if (!json_is_object(json))
throw ConfigError(json, "node-config-node",
"Node configuration must be an object");

Expand Down
4 changes: 3 additions & 1 deletion packaging/docker/Dockerfile.debian
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,9 @@ RUN apt-get update && \
liblua5.3-dev \
libhiredis-dev \
libnice-dev \
libmodbus-dev
libmodbus-dev\
python3-dev \
python3-pybind11

# Install unpackaged dependencies from source
ADD packaging/patches /deps/patches
Expand Down
4 changes: 3 additions & 1 deletion packaging/docker/Dockerfile.debian-multiarch
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,9 @@ RUN apt-get update && \
libusb-1.0-0-dev:${ARCH} \
liblua5.3-dev:${ARCH} \
libhiredis-dev:${ARCH} \
libmodbus-dev:${ARCH}
libmodbus-dev:${ARCH} \
python3-dev:${ARCH} \
python3-pybind11:${ARCH}

ADD cmake/toolchains/debian-${ARCH}.cmake /

Expand Down
5 changes: 3 additions & 2 deletions packaging/docker/Dockerfile.fedora
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ RUN dnf -y install \
openssh-clients \
jq nmap-ncat \
iproute iproute-tc \
python python-devel python-pip \
python-devel python-pip \
gdb gdb-gdbserver \
cppcheck \
xmlto dblatex rubygem-asciidoctor \
Expand Down Expand Up @@ -63,7 +63,8 @@ RUN dnf -y install \
lua-devel \
hiredis-devel \
libnice-devel \
libmodbus-devel
libmodbus-devel \
pybind11-devel
# TODO: v1.2.1 seems broken. Re-enable once new version is available
# nanomsg-devel

Expand Down
6 changes: 4 additions & 2 deletions packaging/docker/Dockerfile.rocky
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ RUN dnf --allowerasing -y install \
flex bison \
texinfo git git-svn curl tar patchutils \
protobuf-compiler protobuf-c-compiler \
clang-tools-extra
clang-tools-extra \
python-devel

# Dependencies
RUN dnf -y install \
Expand All @@ -48,7 +49,8 @@ RUN dnf -y install \
lua-devel \
hiredis-devel \
libnice-devel \
libmodbus-devel
libmodbus-devel \
pybind11-devel

# Install unpackaged dependencies from source
ADD packaging/patches /deps/patches
Expand Down
4 changes: 3 additions & 1 deletion packaging/docker/Dockerfile.ubuntu
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,9 @@ RUN apt-get update && \
libmodbus-dev \
libre2-dev \
libglib2.0-dev \
libcriterion-dev
libcriterion-dev \
python3-dev \
python3-pybind11

# Install unpackaged dependencies from source
ADD packaging/patches /deps/patches
Expand Down
10 changes: 9 additions & 1 deletion tests/unit/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -35,5 +35,13 @@ add_custom_target(run-unit-tests
USES_TERMINAL
)

add_custom_target(run-python-binding-tests
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not so sure if this is really a unit-test, as its not testing individual and isolated parts of the code (e.g. functions).

Instead the Python test seems a bit more elaborate.

What do you think about adding it as an integration test? Or maybe as a dedicated test in itself?

I am interested in your point of view here :)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that the mentioned tests extend beyond unit tests.
I would consider splitting them up into unit and integration tests.

While discussing tests, I would like to mention the Python tests in python/villas/node.

The folder structure node/python/villas/node appears a little confusing to me.
That said, I noticed that there seems to be test code as well as production code within that folder.

Therefore I would propose either creating a separate location for Python related tests or to also split them up into unit and integration tests. I believe this would benefit the clarity of the projects structure.

COMMAND
python3 ${CMAKE_SOURCE_DIR}/tests/unit/python_binding.py
DEPENDS
python-binding
USES_TERMINAL
)

add_dependencies(tests unit-tests)
add_dependencies(run-tests run-unit-tests)
add_dependencies(run-tests run-unit-tests run-python-binding-tests)
Loading