hal: C++ API (hal.hh) and pybind11 bindings on the new HAL API#4251
hal: C++ API (hal.hh) and pybind11 bindings on the new HAL API#4251grandixximo wants to merge 4 commits into
Conversation
|
This seems to be working, but I am not sure if it is the right shape we want, these are the direction I took on the issue raised in the meeting, subject to revision.
@BsAtHome A bare hal_sint_t is width-blind, and wrapping foreign raw handles across widths is unsafe in the interim. But the C++ layer never selects from the handle alone. Component-created pins carry the type at construction via traits (int32_t vs int64_t are distinct C++ types, compile-time selection), and the runtime path uses q->pp.type from your query API to pick the variant alternative, the stored-type-tag multiplex you described in #4099. The only unsafe path is explicitly reinterpreting a raw handle, which can be guarded with a query lookup. So the layer works transitionally, it just gets simpler after the break. |
|
oh, that was quick. I think this should be separate from the other big PR, as this can go in master fairly quickly. the only reason I stopped was that buster had no pybind11 package. this should also replace the current pyhal/halmodule instead of adding a third module. and, the same cpp api should be used in xhc, task and other places which are implemented in cpp. |
|
Buster is basically not supported anymore with master, I will fix the review points tomorrow, thank you both |
Reintroduce a C++ interface for HAL, built strictly on the public C API and the user-land query API: no hal_priv.h, no direct shared memory access, no re-implemented library internals. - hal.hh: type-safe, header-only C++ layer. Typed pin/param handles via traits<T> (compile-time accessor selection for the rtapi_ types), runtime-typed pin_t variant and anypin for name-based access, port wrapper, component class, and ULAPI by-name query/set functions when the query API is present. Handles re-read the shmem slot on every access so hal_link() slot rewrites stay visible (C pointer-variable semantics). Query callback paths are exception-free; errors are reported after the library releases the HAL mutex. - halpybind.cc: pybind11 module (halpp.so) exposing component, Pin, enums and the by-name functions. Built alongside _hal/hal.py, replacing nothing. - taskclass converted to the new API (it was the only hal.hh user). - tests/halpp: Python and native C++ smoke suites. Based on the pybind11 branch by rene-dev, re-based on the new HAL API.
a8c2a71 to
87ff92b
Compare
|
There is another issue. The new hal_lib uses a reference counted init/exit and has separated out Calling The python class must initialize using hal_lib_init() and terminate by calling hal_lib_exit(). If the user has not called hal_exit() on any created components, you will now get a proper error message (because the user forgot to terminate correctly). |
|
can you rename the module to halmodule, so that the UIs and the tests that are already in place use the new bindings, and remove the old ones? |
Initialize the user-land HAL library at module import and register hal_lib_exit() with Py_AtExit so forgotten component exits are reported (query API tree only, where these exist). String overloads of set_value/set_p/set_signal delegate text-to-value conversion to setps_common_cb for consistency with halcmd setp/sets (boolean names, locale-safe floats, range checks). setps_util.c is linked via $(wildcard) so the build works with and without it.
- Ports transfer binary data: read/write/peek use std::vector<char>, bindings expose bytes - Port pins have a scalar value: reads yield the queue reference (via hal_get_sint until hal_get_port() lands), writes are rejected - Port creation is core hal lib; comment corrected, still gated on availability of hal_pin_new_port() - By-name section documented user-space-only by definition - Type constants use the new names HAL_BOOL/HAL_REAL throughout - pin_has_writer uses hal_getref_p/hal_getref_s (no value read) - get_value propagates the library error instead of a generic not-found
Replacing halmodule is the goal, agreed, but renaming now would break things for two separate reasons. First, a hard technical one: on master the new bindings cannot cover the existing Python surface. Second, the compatibility tail: So the proposal stands as: land this additive now (hal.hh + halpp alongside, nothing replaced), and do the rename + replacement as a follow-up once #4247 is in. Your old branch's consumer sweep (hal_glib, qtvcp core, raster, tests) is the starting checklist for that PR. Is this acceptable? Open for discussion... |
Port handle type and creation semantics are still in flux (opaque hal_port_t conflicts with the legacy form, hal_get_port is disabled, hal_pin_new_port does not exist in the base tree), so a port wrapper written now would be transitional glue that gets redone at the break. Remove the port class, the variant alternative and the bindings; document the exemption. Ports return post-break as a hal_port_t based wrapper.
|
I see that #4231 changes halmodule.cc, we should also add those functions via c++/pybind11 |
No, all of this should be in halquery. |
3 participants
@rene-dev @BsAtHome - this is the unification pass we discussed in the #4099 meeting: Rene's C++/pybind11 interface rebuilt on Bertho's getter/setter API, satisfying both constraints (no HAL internals access; simple, low-maintenance, self-documenting C++ surface).
Based on master, fully additive:
_hal/hal.pyare untouched and nothing is replaced. The by-name query functions and HAL_PORT creation are compiled out when the query API header is absent and activate automatically when #4247 lands.What this adds
src/hal/hal.hh- type-safe C++ layer, header-only, innamespace linuxcnc::hal:hal::pin<T>typed handles viatraits<T>over the rtapi_ types:rtapi_s32/rtapi_sintare distinct C++ types, so 32/64 accessor selection happens at compile time with zero cost. Handles re-read the shmem slot per access, sohal_link()rewrites stay visible (same semantics as C pin pointers). Handles are move-only; HAL objects are unique.hal::pin_tvariant +hal::anypin: runtime-typed access multiplexes on the variant tag (the stored-type pattern required for name-keyed collections), with the tag supplied by the query API.hal::component: pins + params,newpin/newparam(typed and runtime-typed),add_pinfor the struct-member idiom, item access, prefix handling.get_value,set_value,set_signal,component_exists,pin_has_writer, signal management) implemented entirely onhal_get_p/hal_set_p/hal_get_s/hal_set_s/hal_comp_by_name. Nohal_priv.h, no direct shmem access, no re-implemented parsers.src/hal/halpybind.cc- pybind11 module (halpp.so), adapted from @rene-dev's bindings, extended with params and error paths. Built alongside_hal/hal.py; module replacement is a separate (3.0) discussion.src/emc/task/taskclass.cc- converted to the new API (it was the only hal.hh consumer).tests/halpp/- Python and native C++ smoke suites (both green), pluspython3-pybind11build dep.Safety properties worth noting
Relationship to the original pybind11 branch
Kept: the variant/map/PyPin structure and the binding shape.
Dropped: the private
set_commonstring parser (superseded byhal_set_p),hal_mutex_guard, theget_info_signalsstub,waitWritable(was non-functional).Compatibility aliases (
hal_comp,hal_pin<T>,hal_dir,PyPin) are provided so existing consumers of the old hal.hh keep working.Test evidence
Full tree builds;
tests/halmodule.0/.1pass; new smoke suites cover component lifecycle, typed pins/params (incl. negative S32 and 64-bit values), signals, linking, by-name get/set with coercion, port pins, and error paths. The suites auto-skip query-API checks when built without the query API, so they run on both master and the #4247 tree.