Skip to content

Conversation

QuantuMope
Copy link
Contributor

This PR adds an automated build script for wx_armor. It's quite annoying / tedious to have to install it on fresh machines. This script should now handle all necessary dependencies seamlessly.

Tested on two separate machines.

An old tutorial can be found here for reference.
https://horizonrobotics.feishu.cn/wiki/NYL6wgBc8i05O0kLkk2cqFNxn2g

Copy link
Contributor

@le-horizon le-horizon left a comment

Choose a reason for hiding this comment

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

Thanks for the nice PR. A couple suggestions if they make sense.

README.md Outdated

Users must also set `LD_LIBRARY_PATH` to the shared library locations. In your `~/.bashrc`, add the following line:
```bash
export LD_LIBRARY_PATH=/usr/local/lib:$LD_LIBRARY_PATH
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe add a check in build.sh to add this to .bashrc if not already there?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried doing this, but since this build script is being run using sudo, it uses the root's .bashrc instead. Since there's no easy fix for this, I simply omitted it. Instead, I have the part about setting LD_LIBRARY_PATH be the first step in README.

- `sudo apt install -y yaml-cpp-dev`

#### Building From Source

Copy link
Contributor

Choose a reason for hiding this comment

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

Add some explanation about the system requirements, e.g. ubuntu version tested etc..
Also add a link to our other repo for running on Jetson devices.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. Done.

Copy link
Contributor Author

@QuantuMope QuantuMope left a comment

Choose a reason for hiding this comment

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

Hey Le, thanks for the review. Addressed your comments. PTAL.

I can also confirm that the build script works flawlessly on Ubuntu 24.04, in addition to 22.04.

- `sudo apt install -y yaml-cpp-dev`

#### Building From Source

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. Done.

README.md Outdated

Users must also set `LD_LIBRARY_PATH` to the shared library locations. In your `~/.bashrc`, add the following line:
```bash
export LD_LIBRARY_PATH=/usr/local/lib:$LD_LIBRARY_PATH
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried doing this, but since this build script is being run using sudo, it uses the root's .bashrc instead. Since there's no easy fix for this, I simply omitted it. Instead, I have the part about setting LD_LIBRARY_PATH be the first step in README.

le-horizon
le-horizon previously approved these changes Dec 27, 2024
Copy link
Contributor

@le-horizon le-horizon left a comment

Choose a reason for hiding this comment

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

LG. Btw, is there any instruction about udev rules? wx_armor fails to talk to the arm w/o the udev rules to define /dev/ttyDXL

@QuantuMope QuantuMope force-pushed the PR/andrew/build_script branch from 7e5238d to 89a076f Compare August 25, 2025 19:40
@QuantuMope
Copy link
Contributor Author

Forced pushed a rebase to main @le-horizon

Copy link
Contributor

@le-horizon le-horizon left a comment

Choose a reason for hiding this comment

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

Thanks for the automation. Some suggestions to make the process even more automatic.

build.sh Outdated
Comment on lines 64 to 76
# Install json
cd /tmp
rm -rf json
git clone https://github.com/nlohmann/json
cd json
mkdir build
cd build
cmake ..
make -j"$(nproc)"
make install
cd /tmp
rm -rf json
cd "$HOME_DIR"
Copy link
Contributor

Choose a reason for hiding this comment

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

This will do:
sudo apt-get install -y nlohmann-json3-dev

Also need these:
sudo apt-get install -y udev libyaml-cpp-dev libspdlog-dev gcc g++ cmake libjsoncpp-dev uuid-dev zlib1g-dev

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@le-horizon
Copy link
Contributor

@QuantuMope could you work on this PR so we can close #33?

Also need to avoid using export LD_LIBRARY_PATH=/usr/local/lib:$LD_LIBRARY_PATH, but instead improve the build script. This env var is perhaps one of the last few things toward a fully automated install.

@QuantuMope QuantuMope force-pushed the PR/andrew/build_script branch 2 times, most recently from f1cfc45 to 5cddb5e Compare September 5, 2025 18:14
Copy link
Contributor Author

@QuantuMope QuantuMope left a comment

Choose a reason for hiding this comment

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

Hey Le, addressed all comments. Also, added a cmake fix to avoid the .bashrc LD_LIBRARY_PATH setting.

Tested that everything works on gail7.

build.sh Outdated
Comment on lines 64 to 76
# Install json
cd /tmp
rm -rf json
git clone https://github.com/nlohmann/json
cd json
mkdir build
cd build
cmake ..
make -j"$(nproc)"
make install
cd /tmp
rm -rf json
cd "$HOME_DIR"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@QuantuMope QuantuMope force-pushed the PR/andrew/build_script branch from b59ae20 to 22fcc66 Compare September 5, 2025 18:18
Copy link
Contributor

@le-horizon le-horizon left a comment

Choose a reason for hiding this comment

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

Just one question. Thanks for the automation!

spdlog::spdlog
nlohmann_json::nlohmann_json
yaml-cpp)
yaml-cpp::yaml-cpp)
Copy link
Contributor

Choose a reason for hiding this comment

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

dynamixel_sdk dependency seems to be missing, have you tested CMake w/o LD_LIBRARY_PATH?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I have. I unset it. I don't think that dependency existed before in the cmake. Could be that toolbox refers to sdk in its own cmake.

@QuantuMope QuantuMope merged commit 515a8d1 into main Sep 5, 2025
@QuantuMope QuantuMope deleted the PR/andrew/build_script branch September 5, 2025 19:09
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.

2 participants