Apollo 7.0 Planning Header Timestamp Bug

[YuqiHuai]

Describe the bug

The planning component modifies the header timestamp after the planning trajectory has been produced, and modifies relative timestamp for each path point in the trajectory.

However, in Apollo 7.0, because variable start_time was assigned at an incorrect location (after header timestamp already changed), variable dt is always 0. As a result, planning_component incorrectly modified the planning message that should be published to CyberRT.

https://github.com/ApolloAuto/apollo/blob/463fb82f9e979d02dcb25044e60931293ab2dba0/modules/planning/planning_component.cc#L188-L194

This bug was fixed in Apollo 8.0

https://github.com/ApolloAuto/apollo/blob/3ecbf3006d9a1c3d832c3d925dc838d2d762c571/modules/planning/planning_component.cc#L188-L194

Question

Since the trajectory as part of the planning module uses timestamp relative to the header timestamp, the output trajectory are essentially equivalent from the perspective of absolute time if (1) both header and relative time changes by a constant dt, or (2) both header and relative time remain unchanged (e.g., after FillHeader, revert the change to header timestamp).

Is there a specific reason why option (1) was chosen over option (2)?

[YuqiHuai]

@qwetqwe I found your fix commit at 9c764ed. Do you mind providing some feedback on if I can choose to not modify the relative times in the trajectory but revert header timestamp change?

 ADCTrajectory adc_trajectory_pb; 
 planning_base_->RunOnce(local_view_, &adc_trajectory_pb); 
 auto start_time = adc_trajectory_pb.header().timestamp_sec(); 
 common::util::FillHeader(node_->Name(), &adc_trajectory_pb); 

 // modify trajectory relative time due to the timestamp change in header 
 // const double dt = start_time - adc_trajectory_pb.header().timestamp_sec(); 

 // revert header timestamp change
 adc_trajectory_pb->mutable_header()->set_timestamp_sec(start_time);