Skip to content
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.

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

feat: load global parameter #26

Merged
merged 4 commits into from
Feb 24, 2022
Merged

feat: load global parameter #26

merged 4 commits into from
Feb 24, 2022

Conversation

wep21
Copy link
Contributor

@wep21 wep21 commented Feb 24, 2022

Signed-off-by: wep21 <border_goldenmarket@yahoo.co.jp>
Signed-off-by: wep21 <border_goldenmarket@yahoo.co.jp>
Signed-off-by: wep21 <border_goldenmarket@yahoo.co.jp>
@wep21 wep21 requested a review from yukkysaito February 24, 2022 06:20
yukkysaito
yukkysaito previously approved these changes Feb 24, 2022
@yukkysaito
Copy link
Collaborator

@wep21 Can you change in the perception.launch?

kenji-miyake
kenji-miyake previously approved these changes Feb 24, 2022
Copy link
Contributor

@kenji-miyake kenji-miyake left a comment

Choose a reason for hiding this comment

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

I feel you can rename global_param to gp or something short to simplify the code, but LGTM.

@kenji-miyake
Copy link
Contributor

Also, considering this use case, it might be useful if we put parameters under vehicle_info/ namespace.

@wep21
Copy link
Contributor Author

wep21 commented Feb 24, 2022

Can you change in the perception.launch?

Sure!

@yukkysaito
Copy link
Collaborator

yukkysaito commented Feb 24, 2022

Also, considering this use case, it might be useful if we put parameters under vehicle_info/ namespace.

Exactly
Why does it work without vehicle_info namespace?

@yukkysaito
Copy link
Collaborator

Currently , do we not add vehicle_info namespace?

@kenji-miyake
Copy link
Contributor

Currently , do we not add vehicle_info namespace?

No, we don't.

Signed-off-by: wep21 <border_goldenmarket@yahoo.co.jp>
@wep21 wep21 dismissed stale reviews from kenji-miyake and yukkysaito via de2fb8b February 24, 2022 06:41
@wep21
Copy link
Contributor Author

wep21 commented Feb 24, 2022

I feel you can rename global_param to gp or something short to simplify the code, but LGTM.

fixed at de2fb8b

@wep21 wep21 requested a review from kenji-miyake February 24, 2022 06:42
@yukkysaito
Copy link
Collaborator

yukkysaito commented Feb 24, 2022

No, we don't.

Oh...Seriously

@wep21 wep21 merged commit ea5525a into tier4/universe Feb 24, 2022
@wep21 wep21 deleted the load-global-param branch February 24, 2022 06:45
@kenji-miyake
Copy link
Contributor

@yukkysaito Yes because there are no conflicts of parameter names.

@yukkysaito
Copy link
Collaborator

ops, I deleted comment

h-ohta pushed a commit that referenced this pull request Nov 22, 2022
* sync develop for main (#46)

* use dual return outlier filter for x2(#24)

* expand crop area (#26)

Co-authored-by: Takeshi Miura <57553950+1222-takeshi@users.noreply.github.com>
Co-authored-by: Shinnosuke Hirakawa <8327162+0x126@users.noreply.github.com>

* Bump streetsidesoftware/cspell-action from 1.3.4 to v1 (#50)

* Bump streetsidesoftware/cspell-action from 1.3.4 to 1.3.6

Bumps [streetsidesoftware/cspell-action](https://github.com/streetsidesoftware/cspell-action) from 1.3.4 to 1.3.6.
- [Release notes](https://github.com/streetsidesoftware/cspell-action/releases)
- [Changelog](https://github.com/streetsidesoftware/cspell-action/blob/main/CHANGELOG.md)
- [Commits](streetsidesoftware/cspell-action@v1.3.4...v1.3.6)

---
updated-dependencies:
- dependency-name: streetsidesoftware/cspell-action
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>

* Update spell_check_pr.yml

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Kenji Miyake <31987104+kenji-miyake@users.noreply.github.com>

Co-authored-by: Kosuke Takeuchi <kosuke.tnp@gmail.com>
Co-authored-by: Takeshi Miura <57553950+1222-takeshi@users.noreply.github.com>
Co-authored-by: Shinnosuke Hirakawa <8327162+0x126@users.noreply.github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Kenji Miyake <31987104+kenji-miyake@users.noreply.github.com>
h-ohta pushed a commit that referenced this pull request Nov 22, 2022
* feat: load global parameter

Signed-off-by: wep21 <border_goldenmarket@yahoo.co.jp>

* ci: run pre-commit

Signed-off-by: wep21 <border_goldenmarket@yahoo.co.jp>

* refactor: address review

Signed-off-by: wep21 <border_goldenmarket@yahoo.co.jp>

* chore: cosmetic change

Signed-off-by: wep21 <border_goldenmarket@yahoo.co.jp>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants