Skip to content

hal: Fix implementation types and small adjustments#4260

Merged
grandixximo merged 1 commit into
LinuxCNC:masterfrom
BsAtHome:halgs_structural-missing
Jul 23, 2026
Merged

hal: Fix implementation types and small adjustments#4260
grandixximo merged 1 commit into
LinuxCNC:masterfrom
BsAtHome:halgs_structural-missing

Conversation

@BsAtHome

Copy link
Copy Markdown
Contributor

In this PR:

  • Fix some types (still all compatible). The function signatures of the new functions were correct, but the implementations lagged.
  • A long standing bug in hal_link would not properly copy the 64-bit value from the dummy to the signal, but would only set the lower 32-bit value. This has now been corrected.
  • Final preparations are put in place for introducing the HAL query API.
  • Update the comment in hal_init_data regarding the mutex.

This PR introduces hal_strerror() for textual representation of the negative error value from many of the HAL functions. The return value of some HAL functions will be adapted to become more fine-grained later. Currently, they mostly return -EINVAL, which often does not describe the actual error encountered. Changing the return value should not pose any problem because the code that actually checks return values looks for equality with zero or less than zero. Returning a more differentiating (negative) value does not functionally alter that code but allows better messages to be generated moving forward.

Comment thread src/hal/hal_lib.c
Comment thread src/hal/hal_lib.c
Comment thread src/hal/hal_lib.c
@grandixximo

Copy link
Copy Markdown
Contributor

If merge this first my #4251 will fail to build, can you provide an explicit marker like

#define HAL_HAS_QUERY_API 1

And I re-gate #4251 on that?

@BsAtHome

Copy link
Copy Markdown
Contributor Author

If merge this first my #4251 will fail to build, can you provide an explicit marker like

Why and where does that fail?

@BsAtHome
BsAtHome force-pushed the halgs_structural-missing branch from 024ef5d to b44b049 Compare July 23, 2026 06:46
Comment thread src/hal/hal.h
@BsAtHome

Copy link
Copy Markdown
Contributor Author

BTW, the next PR will provide the HAL query API. That should make most changes to these parts complete.

@grandixximo

grandixximo commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

If merge this first my #4251 will fail to build, can you provide an explicit marker like

Why and where does that fail?

I will double check the claim in a few hours once I'm on a linux machine

arm sid is broken, are we closing an eye on that?

@BsAtHome

Copy link
Copy Markdown
Contributor Author

And now CI/Sid is acting up again. Sid has been very unstable lately.

@BsAtHome

Copy link
Copy Markdown
Contributor Author

I will double check the claim in a few hours once I'm on a linux machine

Haven't used anything else for 28 years :-)

arm sid is broken, are we closing an eye on that?

Hm, our messages crossed. Email notify has not yet arrived. Sometimes...

@grandixximo

Copy link
Copy Markdown
Contributor

I don't sid being obviously broken should stall us...

@BsAtHome

Copy link
Copy Markdown
Contributor Author

I don't sid being obviously broken should stall us...

I don't think it is a problem either.

@grandixximo

grandixximo commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

If merge this first my #4251 will fail to build, can you provide an explicit marker like

Why and where does that fail?

It fails because of how I detect the query API in #4251. My by-name code (hal_getref_p etc.) only exists once the query API PR lands, so I gated it on

#if defined(COMPONENT_TYPE_REALTIME)

using the enum as a proxy for "query API is present", because your query PR was the one that introduced that macro in the public header.

This PR moves COMPONENT_TYPE_* into public hal.h on its own, ahead of the query API. From that point the macro is defined but hal_getref_p and friends do not exist yet, so my gate opens too early and hal.hh fails to compile:

src/hal/hal.hh:448:64: error: 'hal_query_value_u' has not been declared

Verified on a test branch: this PR + my #4251 commit cherry-picked on top.

@BsAtHome

Copy link
Copy Markdown
Contributor Author

It fails because of how I detect the query API in #4251. My by-name code (hal_getref_p etc.) only exists once the query API PR lands, so I gated it on
#if defined(COMPONENT_TYPE_REALTIME)

That is a wrong assumption. You should not assume any of these defines being there. Your code trying to figure out whether the query code is present needs to be removed anyway.

FWIW, the component type defines are moved to hal.h because they are needed for the query API. This PR is prep work. The next PR will make sense of that again. By that time, your "hack" has become redundant. So, just wait it out for a moment. It is already hard enough not to break master. Trying to keep your not-presently-merged-and-needs-to-change-anyway code running at the same time makes life very difficult. ;-)

@grandixximo
grandixximo merged commit e78a580 into LinuxCNC:master Jul 23, 2026
60 of 64 checks passed
@grandixximo

Copy link
Copy Markdown
Contributor

Fair, don't want to overcomplicate things, we'll hold on #4251

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants