Skip to content

fix(nebula_hw_interfaces_hesai): correct ptp config format for AT128P tcp api #360

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
Ayrton2718 wants to merge 3 commits into
base: main
Choose a base branch
from
Open

fix(nebula_hw_interfaces_hesai): correct ptp config format for AT128P tcp api #360

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e1f9890
feat(hesai): refactor PTP config handling for AT128.
Ayrton2718 Sep 25, 2025
e058d7e
ci(pre-commit): autofix
pre-commit-ci[bot] Sep 25, 2025
abbb152
Merge branch 'beta/v0.3.2.2' into fix/at128-ptp-config
Ayrton2718 May 12, 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
1 change: 0 additions & 1 deletion ...esai/nebula_hesai_hw_interfaces/include/nebula_hesai_hw_interfaces/hesai_cmd_response.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -951,7 +951,6 @@ struct HesaiPtpConfig1588v2 : public HesaiPtpConfigBase
struct HesaiPtpConfig1588v2Extended : public HesaiPtpConfig1588v2
{
uint8_t reserved = 0;
// No extra fields to show in JSON currently, but good to have the struct for potential future use
};

struct HesaiPtpConfig8021AS : public HesaiPtpConfigBase
Expand Down
219 changes: 103 additions & 116 deletions src/nebula_hesai/nebula_hesai_hw_interfaces/src/hesai_hw_interface.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -748,37 +748,47 @@ Status HesaiHwInterface::set_ptp_config(
return Status::ERROR_1;
}

// Handle the OT128 differently - it has TSN settings and defines the PTP profile
// for automotive as 0x03 instead of 0x02 for other sensors.
if (sensor_configuration_->sensor_model == SensorModel::HESAI_PANDAR128_E4X) {
if (profile != static_cast<int>(PtpProfile::IEEE_802_1AS_AUTO)) {
return Status::SENSOR_CONFIG_ERROR;
}
profile = 3;
}

std::vector<unsigned char> request_payload;
request_payload.emplace_back(profile & 0xff);
request_payload.emplace_back(domain & 0xff);
request_payload.emplace_back(network & 0xff);

if (profile == 0) {
request_payload.emplace_back(logAnnounceInterval & 0xff);
request_payload.emplace_back(logSyncInterval & 0xff);
request_payload.emplace_back(logMinDelayReqInterval & 0xff);

// Check if we need to append the reserved byte (New Generation sensors)
bool is_new_gen =
(sensor_configuration_->sensor_model == SensorModel::HESAI_PANDARQT128 ||
sensor_configuration_->sensor_model == SensorModel::HESAI_PANDARAT128 ||
sensor_configuration_->sensor_model == SensorModel::HESAI_PANDAR128_E4X);

if (is_new_gen) {
request_payload.emplace_back(0); // Reserved byte
// Handle different sensor models with different PTP config formats
switch (sensor_configuration_->sensor_model) {
case SensorModel::HESAI_PANDARAT128: {
if (profile != static_cast<int>(PtpProfile::IEEE_802_1AS_AUTO)) {
return Status::SENSOR_CONFIG_ERROR;
}
// AT128 uses status, profile, domain, network, tsn_switch format
request_payload.emplace_back(1); // status: enabled
request_payload.emplace_back(profile & 0xff);
request_payload.emplace_back(domain & 0xff);
request_payload.emplace_back(network & 0xff);
request_payload.emplace_back(switch_type & 0xff); // tsn_switch
break;
}
case SensorModel::HESAI_PANDAR128_E4X: {
// OT128 uses profile 3 for automotive on the wire
if (profile != static_cast<int>(PtpProfile::IEEE_802_1AS_AUTO)) {
return Status::SENSOR_CONFIG_ERROR;
}
int ot128_profile = 3;
request_payload.emplace_back(ot128_profile & 0xff);
request_payload.emplace_back(domain & 0xff);
request_payload.emplace_back(network & 0xff);
request_payload.emplace_back(switch_type & 0xff);
break;
}
default: {
request_payload.emplace_back(profile & 0xff);
request_payload.emplace_back(domain & 0xff);
request_payload.emplace_back(network & 0xff);
if (profile == 0) {
request_payload.emplace_back(logAnnounceInterval & 0xff);
request_payload.emplace_back(logSyncInterval & 0xff);
request_payload.emplace_back(logMinDelayReqInterval & 0xff);
} else if (profile == 2 || profile == 3) {
request_payload.emplace_back(switch_type & 0xff);
}
break;
}

} else if (profile == 2 || profile == 3) {
request_payload.emplace_back(switch_type & 0xff);
}

auto response_or_err = send_receive(g_ptc_command_set_ptp_config, request_payload);
Expand All @@ -801,13 +811,7 @@ std::shared_ptr<HesaiPtpConfigBase> HesaiHwInterface::get_ptp_config()
uint8_t domain = response[2];
uint8_t network = response[3];

// Base fields are 4 bytes.

if (profile == 0) {
// 1588v2
// Check payload size to determine if it's extended or legacy
// Legacy: 4 + 3 = 7 bytes
// Extended: 4 + 4 = 8 bytes
if (response.size() >= 8) {
auto config = std::make_shared<HesaiPtpConfig1588v2Extended>();
config->status = status;
Expand All @@ -831,8 +835,6 @@ std::shared_ptr<HesaiPtpConfigBase> HesaiHwInterface::get_ptp_config()
return config;
}
} else if (profile == 2 || profile == 3) {
// 802.1AS
// Expect 4 + 1 = 5 bytes
if (response.size() >= 5) {
auto config = std::make_shared<HesaiPtpConfig8021AS>();
config->status = status;
Expand Down Expand Up @@ -1189,95 +1191,80 @@ HesaiStatus HesaiHwInterface::check_and_set_config(
std::this_thread::sleep_for(wait_time);
}

if (sensor_configuration->sensor_model != SensorModel::HESAI_PANDARAT128) {
set_flg = true;
Copy link

Copilot AI Sep 26, 2025

Choose a reason for hiding this comment

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

The removal of the conditional check if (sensor_configuration->sensor_model != SensorModel::HESAI_PANDARAT128) means this code now executes for all sensor models including AT128. This could be problematic if AT128 doesn't support the sync angle configuration that follows, as the original code specifically excluded AT128 from this block.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Author

@Ayrton2718 Ayrton2718 Sep 29, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

According to the spec sheet, the AT128 supports sync angle configuration.
I also tested it on a real device.

AT128P_TCP_API_1.4.pdf

Copy link
Copy Markdown
Collaborator

@mojomex mojomex Sep 29, 2025

Choose a reason for hiding this comment

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

Ah, it seems that you are trying to run Nebula with Hesai AT128P, not the older AT128. We currently do not officially support AT128P since we don't have access to the sensor, and since it's not used in any of our projects.

If your change is all it takes, and you can provide a before/after (best as short videos of you trying to launch Nebula and showing a pointcloud), then we'd be more than happy to support it.

Related to #341.

auto sensor_sync_angle = static_cast<int>(hesai_config.sync_angle.value() / 100);
auto config_sync_angle = sensor_configuration->sync_angle;
int sync_flg = 1;
if (config_sync_angle != sensor_sync_angle) {
set_flg = true;
auto sensor_sync_angle = static_cast<int>(hesai_config.sync_angle.value() / 100);
auto config_sync_angle = sensor_configuration->sync_angle;
int sync_flg = 1;
if (config_sync_angle != sensor_sync_angle) {
set_flg = true;
}
if (sync_flg && set_flg) {
logger_->info("current lidar sync: " + std::to_string(hesai_config.sync));
logger_->info("current lidar sync_angle: " + std::to_string(sensor_sync_angle));
logger_->info("current configuration sync_angle: " + std::to_string(config_sync_angle));
std::thread t(
[this, sync_flg, config_sync_angle] { set_sync_angle(sync_flg, config_sync_angle); });
t.join();
std::this_thread::sleep_for(wait_time);
}
if (sync_flg && set_flg) {
logger_->info("current lidar sync: " + std::to_string(hesai_config.sync));
logger_->info("current lidar sync_angle: " + std::to_string(sensor_sync_angle));
logger_->info("current configuration sync_angle: " + std::to_string(config_sync_angle));
std::thread t(
[this, sync_flg, config_sync_angle] { set_sync_angle(sync_flg, config_sync_angle); });
t.join();
std::this_thread::sleep_for(wait_time);
}

std::thread t([this, sensor_configuration] {
if (
sensor_configuration->sensor_model == SensorModel::HESAI_PANDAR40P ||
sensor_configuration->sensor_model == SensorModel::HESAI_PANDAR64 ||
sensor_configuration->sensor_model == SensorModel::HESAI_PANDARQT64 ||
sensor_configuration->sensor_model == SensorModel::HESAI_PANDARXT16 ||
sensor_configuration->sensor_model == SensorModel::HESAI_PANDARXT32 ||
sensor_configuration->sensor_model == SensorModel::HESAI_PANDARXT32M) {
logger_->info("Trying to set Clock source to PTP");
set_clock_source(g_hesai_lidar_ptp_clock_source);
}

std::thread t([this, sensor_configuration] {
if (
sensor_configuration->sensor_model == SensorModel::HESAI_PANDAR40P ||
sensor_configuration->sensor_model == SensorModel::HESAI_PANDAR64 ||
sensor_configuration->sensor_model == SensorModel::HESAI_PANDARQT64 ||
sensor_configuration->sensor_model == SensorModel::HESAI_PANDARXT16 ||
sensor_configuration->sensor_model == SensorModel::HESAI_PANDARXT32 ||
sensor_configuration->sensor_model == SensorModel::HESAI_PANDARXT32M) {
logger_->info("Trying to set Clock source to PTP");
set_clock_source(g_hesai_lidar_ptp_clock_source);
}
auto current_ptp = get_ptp_config();
bool needs_ptp_update = !ptp_config_matches_desired(*current_ptp, *sensor_configuration);

if (needs_ptp_update) {
std::ostringstream tmp_ostringstream;
tmp_ostringstream << "Trying to set PTP Config: " << sensor_configuration->ptp_profile
<< ", Domain: " << std::to_string(sensor_configuration->ptp_domain)
<< ", Transport: " << sensor_configuration->ptp_transport_type
<< ", Switch Type: " << sensor_configuration->ptp_switch_type << " via TCP";
logger_->info(tmp_ostringstream.str());
set_ptp_config(
static_cast<int>(sensor_configuration->ptp_profile), sensor_configuration->ptp_domain,
static_cast<int>(sensor_configuration->ptp_transport_type),
static_cast<int>(sensor_configuration->ptp_switch_type), g_ptp_log_announce_interval,
g_ptp_sync_interval, g_ptp_log_min_delay_interval);
}
logger_->debug("Setting properties done");
});
logger_->debug("Waiting for thread to finish");

auto current_ptp = get_ptp_config();
bool needs_ptp_update = !ptp_config_matches_desired(*current_ptp, *sensor_configuration);

if (needs_ptp_update) {
std::ostringstream tmp_ostringstream;
tmp_ostringstream << "Trying to set PTP Config: " << sensor_configuration->ptp_profile
<< ", Domain: " << std::to_string(sensor_configuration->ptp_domain)
<< ", Transport: " << sensor_configuration->ptp_transport_type
<< ", Switch Type: " << sensor_configuration->ptp_switch_type
<< " via TCP";
logger_->info(tmp_ostringstream.str());
set_ptp_config(
static_cast<int>(sensor_configuration->ptp_profile), sensor_configuration->ptp_domain,
static_cast<int>(sensor_configuration->ptp_transport_type),
static_cast<int>(sensor_configuration->ptp_switch_type), g_ptp_log_announce_interval,
g_ptp_sync_interval, g_ptp_log_min_delay_interval);
}
logger_->debug("Setting properties done");
});
logger_->debug("Waiting for thread to finish");
t.join();
logger_->debug("Thread finished");

t.join();
logger_->debug("Thread finished");

switch (sensor_configuration_->sensor_model) {
case SensorModel::HESAI_PANDAR128_E4X:
case SensorModel::HESAI_PANDARQT128:
case SensorModel::HESAI_PANDARXT16:
case SensorModel::HESAI_PANDARXT32:
case SensorModel::HESAI_PANDARXT32M: {
uint8_t sensor_ptp_lock_threshold = get_ptp_lock_offset();
if (sensor_ptp_lock_threshold != sensor_configuration_->ptp_lock_threshold) {
NEBULA_LOG_STREAM(
logger_->info, "changing sensor PTP lock offset from "
<< static_cast<int>(sensor_ptp_lock_threshold) << " to "
<< static_cast<int>(sensor_configuration_->ptp_lock_threshold));
set_ptp_lock_offset(sensor_configuration_->ptp_lock_threshold);
}
break;
switch (sensor_configuration_->sensor_model) {
case SensorModel::HESAI_PANDAR128_E4X:
case SensorModel::HESAI_PANDARQT128:
case SensorModel::HESAI_PANDARXT16:
case SensorModel::HESAI_PANDARXT32:
case SensorModel::HESAI_PANDARXT32M: {
uint8_t sensor_ptp_lock_threshold = get_ptp_lock_offset();
if (sensor_ptp_lock_threshold != sensor_configuration_->ptp_lock_threshold) {
NEBULA_LOG_STREAM(
logger_->info, "changing sensor PTP lock offset from "
<< static_cast<int>(sensor_ptp_lock_threshold) << " to "
<< static_cast<int>(sensor_configuration_->ptp_lock_threshold));
set_ptp_lock_offset(sensor_configuration_->ptp_lock_threshold);
}
default:
break;
break;
}

std::this_thread::sleep_for(wait_time);
} else { // AT128 only supports PTP setup via HTTP
logger_->info("Trying to set SyncAngle via HTTP");
set_sync_angle_sync_http(1, sensor_configuration->sync_angle);
std::ostringstream tmp_ostringstream;
tmp_ostringstream << "Trying to set PTP Config: " << sensor_configuration->ptp_profile
<< ", Domain: " << sensor_configuration->ptp_domain
<< ", Transport: " << sensor_configuration->ptp_transport_type << " via HTTP";
logger_->info(tmp_ostringstream.str());
set_ptp_config_sync_http(
static_cast<int>(sensor_configuration->ptp_profile), sensor_configuration->ptp_domain,
static_cast<int>(sensor_configuration->ptp_transport_type), g_ptp_log_announce_interval,
g_ptp_sync_interval, g_ptp_log_min_delay_interval);
default:
break;
}

std::this_thread::sleep_for(wait_time);

if (
sensor_configuration->sensor_model == SensorModel::HESAI_PANDAR128_E3X ||
sensor_configuration->sensor_model == SensorModel::HESAI_PANDAR128_E4X) {
Expand Down
Loading