Skip to content

Fix/camera config loading #34

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

Open
musyaree wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fix/camera config loading #34

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6776bfc
fix: fix config overwrite and auto mode
musyaree May 15, 2026
821d444
fix: wait for camera to be ready
musyaree May 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 28 additions & 7 deletions src/shisen_cpp/camera/node/camera_node.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,10 +197,12 @@ CaptureSetting CameraNode::on_configure_capture_setting(
}

if (new_capture_setting.temperature.is_not_empty()) {
video_capture->set(cv::CAP_PROP_AUTO_WB, 0);
video_capture->set(cv::CAP_PROP_WB_TEMPERATURE, new_capture_setting.temperature);
}

if (new_capture_setting.exposure.is_not_empty()) {
video_capture->set(cv::CAP_PROP_AUTO_EXPOSURE, 1);
video_capture->set(cv::CAP_PROP_EXPOSURE, new_capture_setting.exposure);
}

Expand All @@ -209,13 +211,30 @@ CaptureSetting CameraNode::on_configure_capture_setting(
}
}

// Get new capture setting from the camera
new_capture_setting.brightness = video_capture->get(cv::CAP_PROP_BRIGHTNESS);
new_capture_setting.contrast = video_capture->get(cv::CAP_PROP_CONTRAST);
new_capture_setting.saturation = video_capture->get(cv::CAP_PROP_SATURATION);
new_capture_setting.temperature = video_capture->get(cv::CAP_PROP_WB_TEMPERATURE);
new_capture_setting.exposure = video_capture->get(cv::CAP_PROP_EXPOSURE);
new_capture_setting.gain = video_capture->get(cv::CAP_PROP_GAIN);
// Get new capture setting from the camera if they were empty
if (new_capture_setting.brightness.is_empty()) {
new_capture_setting.brightness = video_capture->get(cv::CAP_PROP_BRIGHTNESS);
}

if (new_capture_setting.contrast.is_empty()) {
new_capture_setting.contrast = video_capture->get(cv::CAP_PROP_CONTRAST);
}

if (new_capture_setting.saturation.is_empty()) {
new_capture_setting.saturation = video_capture->get(cv::CAP_PROP_SATURATION);
}

if (new_capture_setting.temperature.is_empty()) {
new_capture_setting.temperature = video_capture->get(cv::CAP_PROP_WB_TEMPERATURE);
}

if (new_capture_setting.exposure.is_empty()) {
new_capture_setting.exposure = video_capture->get(cv::CAP_PROP_EXPOSURE);
}

if (new_capture_setting.gain.is_empty()) {
new_capture_setting.gain = video_capture->get(cv::CAP_PROP_GAIN);
}
Comment on lines -212 to +237

@FaaizHaikal FaaizHaikal May 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Big No-No on this change. We can't guard these with is_empty(). The camera has its own internal value limits and hardware states, so we must always overwrite our local struct with the actual values coming from video_capture->get(). Please put the old logic back.


return new_capture_setting;
}
Expand All @@ -229,6 +248,8 @@ void CameraNode::configure_capture_setting(const CaptureSetting & capture_settin

void CameraNode::load_configuration(const std::string & path)
{
RCLCPP_INFO_STREAM(node->get_logger(), "Loading configuration from: " << path);

nlohmann::json config;
if (!jitsuyo::load_config(path, "capture_settings.json", config)) {
throw std::runtime_error("Unable to open `" + path + "capture_settings.json`!");
Expand Down
12 changes: 12 additions & 0 deletions src/shisen_cpp/node/shisen_cpp_node.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include <shisen_cpp/node/shisen_cpp_node.hpp>

#include <memory>
#include <thread>

namespace shisen_cpp
{
Expand All @@ -32,6 +33,17 @@ ShisenCppNode::ShisenCppNode(rclcpp::Node::SharedPtr node, const std::string & p
auto image_provider = std::make_shared<camera::ImageProvider>(options);
auto camera_config_provider = std::make_shared<camera::CameraConfigProvider>(options);
camera_node->set_provider(image_provider, camera_config_provider);

for (int i = 0; i < 10; ++i) {
try {
camera_node->update();
break;
} catch (const std::exception & e) {
RCLCPP_WARN(node->get_logger(), "Waiting for camera to be ready...");
std::this_thread::sleep_for(100ms);
}
}

camera_node->load_configuration(path);
Comment on lines +37 to 47

@FaaizHaikal FaaizHaikal May 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

  1. Load the configuration first before spinning up the node to avoid uninitialized variables.

  2. Worst-case scenario, the node will just keep running here even after 10 failures. We should use an initialized flag to explicitly track if the node is ready, and throw an error/abort if it still fails after the retries.


node_timer = node->create_wall_timer(
Expand Down
Loading