@cyn-liu

Please apply the changes not only to the point we pointed out, but to all points related to the comment.

As we said in #7956 (comment), don't use new.
Please check our coding guidelines:

https://autowarefoundation.github.io/autoware-documentation/main/contributing/coding-guidelines/languages/cpp/
Our coding guideline follows CppCoreGuidelines

Refactoring the entire open source project to fully meet the specifications is a significant workload, but I have to do so.
I found that the refactored code often encounters unknown problems during launch, and the cause of this problem has not been found. After restarting the node multiple times, it can be resolved.
Although the previous code did not meet your coding standards, it was simple clear, and can ran stably.

@cyn-liu
Is this what you meant in your comment?

• autoware_tensorrt_bevdet/src/bevdet.cpp was copied from another open source project.
• The content written in the other open source project is being used as is.

If that's the case, instead of modifying the code, the following actions are also possible:

• Clearly indicate where the code was copied from, like in this example.
• Store all copied code in a lib directory.

For code outside the lib, you have to follow Autoware's coding guidelines. For example, Autoware’s coding guidelines recommend avoiding the use of new.

@cyn-liu (cc. @Shin-kyoto)
With all due respect, but

Although the previous code did not meet your coding standards

All PR authors are asked to read and accept the coding standards in the contribution guidelines, so this is a requirement that was clear even before the work on this PR started, so we expect this to be followed. A large open source project cannot be managed otherwise and this is not a concept unique to Autoware.

it was simple clear

I disagree, and so do our coding standards and the C++ core guidelines. Also, this PR is almost 6000 lines which makes even perfectly simple and clear code hard to review thoroughly.

and can ran stably.

As a reviewer, this was impossible to verify due to all the non-standard and almost C-style C++. There is a reason that core guidelines and tools like Clang-Tidy are enforcing the use of smart pointers, disallowing reinterpret_cast etc.
If it does not run stably after the refactoring, that hints at race conditions and memory problems that still need to be addressed. Consider using AddressSanitizer and ThreadSanitizer to pinpoint the issues.

As this review is taking quite some hours to review for our employees, I would strongly suggest that the coding style issues are addressed, and that the PR is split up before occupying people with the same discussion over and over again.

If you do not want to comply with our coding standards, I am afraid we cannot merge this PR.

@cyn-liu

friendly ping

@cyn-liu

friendly ping

@cyn-liu

I have talked with @Shin-kyoto on this.Like @mojomex mentioned, we have a coding standard in AWF which encourages the use of smart pointers instead of normal pointers. Therefore, we would like to comply to the guideline as much as possible.

However, I also understand the effort of refactoring the whole code, especially if the code was taken from another open source. If it is difficult to refactor, alternate option is to wrap the original BEVDet code as is as a vendor package and maintain it outside Autoware with different CIs settings and code standards. Then we can added through autoware.repos or through ROS Buildfarm and just call the libraries from Autoware Nodes. This might also make it easier for us to update the BEVDet implementation if any updates are made upstream.

Do you think the above approach is feasible? It might be difficult if you have already made a lot of modification to the BEVDet implementation so it would be nice if you can provide us how much modification you made from the original code.

I'm also okay to set up an online call if that makes you easier to discuss on this topic.

Good Afternoon, Mits-san @mitsudome-r . thank you for the coordination.

1. Second way is a better option for Continuously introduce great open source project into autoware
2. I will tell Cynthia @cyn-liu to make a list of the modification point here
3. it will be great if we can have a online call for this， especially for the topic “call the libraries from Autoware Nodes”， we are not familiar with this part

have a nice day!
Lucas

The main modifications made to tensorrt-bevdet-cpp library are as follows:

1. Standardize log output by changing print ,std:: cout etc to ros2 rclcpp logger
2. Standardize the names of member functions and variables
3. Modify multiple pointers to cpp std::vector
4. If the engine model does not exist, export engine before inference