docs: update the convention and mention sync-params workflow#804
docs: update the convention and mention sync-params workflow#804paulsohn wants to merge 6 commits into
Conversation
Signed-off-by: Taeseung Sohn <taeseung.sohn@tier4.jp>
|
Thank you for contributing to the Autoware project! 🚧 If your pull request is in progress, switch it to draft mode. Please ensure:
|
|
Documentation URL: https://autowarefoundation.github.io/autoware-documentation/pr-804/ |
technolojin
left a comment
There was a problem hiding this comment.
There are some spoken English, but those are from the original.
For the sync-params process and its policy, I used the process and worked well.
Drawbacks are
- title is not intuitive (because of fixed template)
- parameter update could contain multiple updates of origins(universe, core), which makes hard to cherry-pick for specific combination.
- this sync-params process requires sequential process, so downtime occurs between the original update to this sync. manual process of merging could make almost zero downtime by merge two PRs in the same time.
| In order to automatically load the newly added package (in `autoware_core` or `autoware_universe`) when starting Autoware, you need to make some necessary changes to the corresponding launch file. | ||
| For example, if you want to use ICP instead of NDT as the pointcloud registration algorithm, you can go to `autoware_launch` repository and modify the `tier4_universe_launch/tier4_localization_launch/launch/pose_twist_estimator/pose_twist_estimator.launch.xml` file to load the newly added ICP package. |
There was a problem hiding this comment.
- Instead of showing example and not explain detail, please describe what the modification is.
- topology modification
- replacement of using node
- add a new node, with parameter file registration
- What
"automatically" load the newly added packagemeans?
There was a problem hiding this comment.
It seems the sentences are from the original. Is someone have any idea of this?
There was a problem hiding this comment.
Thank you for your comment. Personally, I guess we can remove the second sentence as this my proposal. Because what we want to say here is, "please fix launch file if you want to launch new package which is added to latest autoware_universe".
Signed-off-by: Taeseung Sohn <taeseung.sohn@tier4.jp>
0c43973 to
cbf853f
Compare
xmfcx
left a comment
There was a problem hiding this comment.
A few editorial fixes from a close read. The first three are content defects (a truncated sentence, a fragment, and a verb that inverts an instruction's meaning); the rest are minor. Left as committable suggestions where possible.
Non-inline note: the mermaid diagram still shows pose_estimator.launch.xml (line 43) while the updated prose now uses pose_twist_estimator — worth aligning for consistency, though it's a pre-existing node outside this diff.
Signed-off-by: Taeseung Sohn <taeseung.sohn@tier4.jp>
Signed-off-by: Taeseung Sohn <taeseung.sohn@tier4.jp>
sasakisasaki
left a comment
There was a problem hiding this comment.
Reviewed. I agree with merging this PR while put some quick win comments 👍
|
@xmfcx @mitsudome-r Could you please review this PR? |
Signed-off-by: Taeseung Sohn <taeseung.sohn@tier4.jp>
Signed-off-by: Taeseung Sohn <taeseung.sohn@tier4.jp>
Description
This updates and clarifies the current conventions:
tier4_*_launchfiles are moved toautoware_launch, since Migratetier4_xxx_launcherfrom autoware_universe into autoware_launch autoware#6774 .Plus minor grammar fixes.
Disclaimer: the current version does not mention any ongoing migrations such as autoware index, and assuming that the autoware_universe is gigantic as it is for now. Can be updated after the migration gain its momentum.