Skip to content

motion: Fix state tag truncation and replace float with double#4020

Open
BsAtHome wants to merge 1 commit into
LinuxCNC:masterfrom
BsAtHome:fix_state-tag-type
Open

motion: Fix state tag truncation and replace float with double#4020
BsAtHome wants to merge 1 commit into
LinuxCNC:masterfrom
BsAtHome:fix_state-tag-type

Conversation

@BsAtHome
Copy link
Copy Markdown
Contributor

The state tag is used in communication between interpreter and motion. Data put into it usually comes from double sized types and got truncated to float size. Afterwards, the values would, for example, be moved to HAL pins and the truncated value would be extended to double again. This is obviously a bad plan.

This PR makes the state tag field_float of double type.

@rodw-au
Copy link
Copy Markdown
Contributor

rodw-au commented May 13, 2026

Need to take care here. What about changes to emcmot_hal_data->interp_feedrate?
Its defined as hal_float_t *interp_feedrate;
Not to mention my pending ones in #3995

    /* New Geometric Metadata Pins */
    hal_float_t *interp_arc_radius;
    hal_float_t *interp_arc_center_x;
    hal_float_t *interp_arc_center_y;
    hal_float_t *interp_arc_center_z;
    hal_float_t *interp_straight_heading;
    hal_float_t *interp_normal_heading;  
    hal_bit_t   *iscircle;

Which leads me to suggest another change to resolve the active_tags mess this touches which I was leaving unitil #3995 was committed.
add an extra float/double array to state_tags
double fields_extra_float[EX_FIELD_MAX_FIELDS]
Move all new pins in #3995 to this field and lock fields_float to the original fields (up to GM_FIELD_FLOAT_NAIVE_CAM_TOLERANCE) with an assert statement and apropiate comments in state_tags.h

@BsAtHome
Copy link
Copy Markdown
Contributor Author

The name hal_float_t is a complete misnomer and not a C-type float. The underlying type is actually a double.

The name "float" is used many places in linuxCNC, but most float types were promoted to double because you need the precision in the calculation. The variable names and defined did not change, only the underlying type.

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