search

RobotWebTools / roslibjs

The Standard ROS JavaScript Library
https://robotwebtools.github.io/roslibjs
Other
677 stars 378 forks source link

Fix undefined reference to "WebSocket" for NodeJS instances #762

Closed mhosmar-cpr closed 1 month ago

mhosmar-cpr commented 2 months ago

Public API Changes None

Description Fixed an undefined reference to WebSocket in src/core/Ros.js. This is a fairly direct approach. We could also rearrange the logic to reference ws.WebSocket.CLOSED on the NodeJS branch. see the issue.

Resolves #756

MatthijsBurgh commented 2 months ago

@EzraBrooks do we want to solve it this way, or do we need to import the websocket class here?

mhosmar-cpr commented 2 months ago

I have implemented the change as described. However I am not sure of the potential cost of repeatedly importing ws. If it does not pose any risk I think this change is good as is.

Unfortunately I don't have the knowledge or time to implement a unit test for this code path. Please understand.

mhosmar-cpr commented 1 month ago

looping back. FYI I can't merge even with approval.

EzraBrooks commented 1 month ago

Running CI and will merge once it passes