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

node-can-bridge: Add support for Linux and macOS #21

Draft
wants to merge 30 commits into
base: main
Choose a base branch
from

Conversation

qwertychouskie
Copy link

The @unofficial-rev-port team has been hard at work making the core technologies used by the REV Hardware Client work on all 3 major platforms (Windows, macOS, Linux). This PR contains changes needed for node-can-bridge to compile and run on macOS and Linux in addition to Windows, including the changes to pull the OS-specific binaries from CANBridge (see REVrobotics/CANBridge#29).

Please note that this PR is still a work-in-progress; while we have successfully ran a local build of the REV Hardware Client that was able to load and initialize the CAN backend, we are still in the process of testing with real hardware to ensure full functionality (and no regressions) on all 3 OSes. We are opening this PR in the current state in order to gather feedback and make sure we do the changes in a way acceptable to having the PR merged upstream (once completed, of course).

Moose1301 and others added 26 commits May 11, 2024 15:21
Tested on M3 Pro Apple Silicon (aarch64)
Manifest workflows, just to get checks of building and releases at least created.
Manifest GitHub Actions workflows
Points to the correct repository now instead of the upstream repository
Added proper support for macOS, Linux, ARM64, and ARM32 based systems.

Rewrote handling artifacts from unzipping, to moving artifacts to correct directories based on what platforms are downloaded and what is currently running on.

Added documentation to helper functions.
New platforms are able to link and build. Tested on macOS.
Enabling exceptions for the compiler causes the project to build without issues, as the compiler would not like having no exception support similar to how macOS operates.
Might need more work but at least it doesn't error
On Windows builds, there was an issue with dependencies not copying and therefore not linking when building native modules, this is now fixed.
Removed duplicated builds and renamed builds of CANBridge.
The stopHeartbeats function was removed in a previous commit, which caused MSVC builds to break. This commit re-adds the function to fix the build. The function will need to be reimplemented with the current codebase, as of right now this does nothing.
Fixes failure on macOS after this was changed elsewhere
Updated compile instructions and formatting.
EXCEPTIONS GO BRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRR it's 3am and i need sleep i spent way too long on this stupid bug but thats how code goes anyways good night
@garrettsummerfi3ld
Copy link

Other notes to be made, we have added CI to this to validate not only the already existing workflows, but also new builds to support Linux and macOS.

Copy link
Member

@NoahAndrews NoahAndrews left a comment

Choose a reason for hiding this comment

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

Thank you for contributing, and seeking feedback early on!

I've left a few comments, but I haven't done a deep dive on this. REVrobotics/CANBridge#29 should probably receive most of your attention for now.

.github/workflows/build.yml Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

There are a lot of strange changes in this file, mostly made in commit b0ca096624286b1e975eaaa816e38599933b7e84 that doesn't explain why they were made. When you are making big changes like that, please explain your reasoning in the commit message, and don't include changes that are unrelated to the purpose of that commit.

Looking at it more, I see that the strange changes are actually just you accidentally undoing all of the changes made to this file in 2024.

Copy link
Author

Choose a reason for hiding this comment

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

Probably a rebase failure, we'll look into it.

Bump node-gyp-build to version 4.8.0 and node-gyp to version 10.1.0 in package.json.
Removal of `setuptools` install step due to bumped `node-gyp` version.
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.

None yet

4 participants