Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Turning ROS2-For-Unity into a package #95

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from
Open

Turning ROS2-For-Unity into a package #95

wants to merge 6 commits into from

Conversation

Deric-W
Copy link
Contributor

@Deric-W Deric-W commented Dec 4, 2023 •
edited
Loading

Hi,
this pull request has the same goal as #94 but with the following extra features:

  • ros2-for-unity.xml is now being packaged by Unity
    • postinstall now only handles versioned shared libraries
    • the metadata file is moved into a resource folder, which seems to be discouraged but since it is only a small xml the addressables would probably be overkill
  • the path and integrity checks are now done by Unity (implies some simple changes when running in edit mode)
    • some methods of ROS2ForUnity where moved to a separate Setup class and marked private or internal where possible
    • since Unity does not call RuntimeInitializeOnLoadMethod marked methods in edit mode the user has to call the method to setup the path manually in this case (for example using OneTimeSetUp in NUnit tests)
  • examples where moved to dedicated package examples
    • they have dedicated scenes which meant the example image could be removed
  • moved Scripts to Runtime directory and add Assembly definition
    • as a side effect internal methods and attributes are now hidden from users
    • some missing .meta files where added but the generated binaries still need to be imported from disk at least once before uploading the package to a registry to generate .meta files for them

If these features are wanted then this PR could replace #94.

@Deric-W
remove scripts for creating an asset package
678eb99
@Deric-W
replace asset with package
047f27a 
This allows better reuse, samples and tests.
Furthermore, the packaging of `ros2-for-unity.xml` is now being
done by Unity as well as setting up the path and doing integrity checks.
@Deric-W
update README
56058af
@Deric-W
add samples
14b87c7 
The old examples where moved into the new ones.
@Deric-W
add missing .meta files
2b84468
@Deric-W
rename Scripts to Runtime
6ddd4ca 
It seems that this is closer to the recommended layout
@RobertoRoos
Copy link

RobertoRoos commented Dec 8, 2023 •
edited
Loading

Looks neat @Deric-W ! Until this gets merged I'll be basing my own forked works on this.

EDIT: Maybe, as the diff is pretty huge, you could complete the list in your first message with all the changes made? I imagine bullets like 'moved Scripts to Runtime' and 'replaced path variables by Resources helper', etc.

@RobertoRoos
Copy link

RobertoRoos commented Dec 11, 2023 •
edited
Loading

@Deric-W I've started using your branch in my own project and it looks like I'm having issues with custom messages. I can compile fine, custom messages are recognized properly. But when running I get UnsatisfiedLinkErrors for DLLs like [message]__rosidl_typesupport_c_native.dll. Do you think you could give it a try too?

EDIT: Oops, fault was mine. Can confirm custom message still work fine as a package!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Deric-W @RobertoRoos