Skip to content

Conversation

@prototriangle
Copy link

Some suppliers provide EDS files with HEX values for the SupportedObjects field.

This patch adds support for this.

@roncapat
Copy link
Contributor

Is this being proposed also upstream?

@prototriangle
Copy link
Author

prototriangle commented Oct 15, 2025

Not currently. Would it be best proposed there?
Is ros2_canopen frequently updated with upstream changes?

@roncapat
Copy link
Contributor

roncapat commented Oct 15, 2025

Disclaimer: I'm not the maintainer here, only active user and observer.

IMHO out-of-tree patches should only be stop-gap measures while either upstream patch is being reviewed -or- some kind of disagreement or lack of support happens again upstream.

So I suggest/encourage you to also propose this patch on lely's gitlab. As it may take some time for this to be accepted, including here temporarily the patch would do the job in the meantime. When it gets upstreamed, another PR here should upgrade lely git hash to a newer one and remove the temporarily applied out-of-tree patch.

@roncapat
Copy link
Contributor

Seems a MR already exists:
https://gitlab.com/lely_industries/lely-core/-/merge_requests/149

That's nice!

@prototriangle
Copy link
Author

Great find. I'll engage on that MR to see if they'd want to add the linting change as well to be the same as this.

@prototriangle
Copy link
Author

The change has now been added to that MR (https://gitlab.com/lely_industries/lely-core/-/merge_requests/149).

Until that moves forward I would be keen to have this as a patch in this repo.

@prototriangle prototriangle marked this pull request as ready for review October 17, 2025 10:55
UPDATE_COMMAND
COMMAND git reset --hard
COMMAND git apply --whitespace=fix --reject ${CMAKE_CURRENT_SOURCE_DIR}/patches/0001-Fix-dcf-tools.patch
COMMAND git apply --whitespace=fix --reject ${CMAKE_CURRENT_SOURCE_DIR}/patches/0002-Allow-base16-values-for-SupportedObjects.patch
Copy link

Choose a reason for hiding this comment

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

I would add a comment above with the lely MR link. That way it's more clear when this patch can be cleaned up again.

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.

3 participants