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.

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

Add Emscripten-wasm32 target #268

Open
wants to merge 108 commits into
base: main
Choose a base branch
from

Conversation

nmarticorena
Copy link

Hi @traversaro @wolfv - this PR implement the changes to rebuild emscripten-wasm32 packages for ros-humble. This is the second and last pull request that relies on the changes on vinca implemented on RoboStack/vinca#73, in addition to the implementation of the corresponding CI pipelines https://github.com/nmarticorena/ros-humble/actions/runs/13779961774 and all the various patches. All the changes were tested and uploaded to my personal Anaconda channel, https://anaconda.org/nmarticorena. With the rebuild of the Jupyter-Lite demo available at: https://nmarticorena.github.io/pixi-wasm/lab/index.html and the source code https://github.com/nmarticorena/pixi-wasm.

It would be great if you could review :).

Best, @Tobias-Fischer and @nmarticorena

@Tobias-Fischer
Copy link
Contributor

Do you mind taking another look please @traversaro?

@Tobias-Fischer Tobias-Fischer marked this pull request as ready for review March 13, 2025 11:03
Comment on lines +9 to +11
git: https://github.com/Tobias-Fischer/rmw_wasm
branch: emscripten-async-fixes
target_directory: ros-humble-rmw-wasm-cpp/src/work
Copy link
Member

Choose a reason for hiding this comment

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

This is a branch (not a tag or commit) and it is on a fork. As I guess this is a work in progress, if you confirm that this is intended it is ok for me.

Copy link
Contributor

Choose a reason for hiding this comment

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

Very good catch! We'll open a pull request against upstream and then update once that's merged.

@traversaro
Copy link
Member

Thanks, I added some comments but mostly minor. My only remaining doubt is if the wasm-related additional recipes are meant to be build on target platforms different from emscripten32 . Could it make sense to add a dummy change to vinca_*.yaml files to check if there are any building failures?

@Tobias-Fischer
Copy link
Contributor

Thanks, I added some comments but mostly minor. My only remaining doubt is if the wasm-related additional recipes are meant to be build on target platforms different from emscripten32 . Could it make sense to add a dummy change to vinca_*.yaml files to check if there are any building failures?

You are absolutely right, good catch. I've added skips for non-emscripten platforms.

@traversaro
Copy link
Member

Great, thanks!

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