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

[jsk_fetch_startup] use plugin for applist load in fetch #1193

Merged
merged 6 commits into from
Jul 16, 2021

Conversation

knorth55
Copy link
Member

@knorth55 knorth55 commented Jan 7, 2020

use plugin for applist load in fetch as PR2

@knorth55
Copy link
Member Author

knorth55 commented Jan 7, 2020

now i'm testing on Fetch15

Copy link
Member

@708yamaguchi 708yamaguchi left a comment

Choose a reason for hiding this comment

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

In my understanding, if you set use_applist arg as false, app directory is loaded from export tag in package.xml.
https://github.com/PR2/app_manager/blob/kinetic-devel/scripts/app_manager#L38

If my understanding is correct, applist arg is no more needed. (Existance of applist arg may be confusing.)
https://github.com/jsk-ros-pkg/jsk_robot/blob/master/jsk_fetch_robot/jsk_fetch_startup/launch/fetch_bringup.launch#L80

@knorth55
Copy link
Member Author

knorth55 commented Jan 8, 2020

yes you are right. we do not need applist arg if use_applist is false.

@k-okada
Copy link
Member

k-okada commented Jan 9, 2020

If I understand correctly, this change load list of apps from jsk_fetch_startapp/apps to export tag. Thus increase the number of apps listed on the main menu. If this is the case.
Please add information on the list of apps before/after this change

@knorth55
Copy link
Member Author

knorth55 commented Jan 9, 2020

So far, all fetch app is located in jsk_fetch_startup/apps, so apps in web menu does not increase.
however, if we add new fetch apps in some repo (like 2019 semi demo in jsk_demos), the new app will appear in web menu.
OK, so I will make 2019 semi program into app.

@k-okada
Copy link
Member

k-okada commented Jan 9, 2020 via email

@knorth55
Copy link
Member Author

knorth55 commented Jan 9, 2020

https://github.com/PR2/app_manager/blob/kinetic-devel/src/app_manager/app_list.py#L131-L136
according to this line, even if we set platform: *, app will be filtered in these lines.
but it is good that we can use platform: * or platform: all.

@knorth55
Copy link
Member Author

knorth55 commented Jan 9, 2020

app like time_signal in #1194 , we can run this app in all robot.

@knorth55
Copy link
Member Author

knorth55 commented Jan 9, 2020

before this PR, all app should be located in jsk_fetch_startup/apps.
For example, welcome_to_jsk app in https://github.com/jsk-ros-pkg/jsk_robot/blob/master/jsk_fetch_robot/jsk_fetch_startup/apps/welcome_to_jsk/welcome_to_jsk.xml is located in jsk_fetch_startup/apps, but all the source is located in jsk_demos.
with this PR, we can avoid this by adding fetch apps in jsk_demos.

@k-okada
Copy link
Member

k-okada commented Jan 9, 2020 via email

@knorth55
Copy link
Member Author

knorth55 commented Jan 9, 2020

this PR is not related to platform: all problem.
this PR is just loading apps from all repo not only from jsk_fetch_startup.

@knorth55
Copy link
Member Author

I made PR to add all platform PR2/app_manager#17

@knorth55 knorth55 mentioned this pull request Feb 11, 2020
29 tasks
@knorth55 knorth55 changed the title use plugin for applist load in fetch [jsk_fetch_startup] use plugin for applist load in fetch Feb 24, 2020
@knorth55 knorth55 mentioned this pull request Jun 19, 2020
12 tasks
@knorth55 knorth55 added this to the 1.1.1 milestone Oct 21, 2020
@k-okada k-okada merged commit 3e2f912 into jsk-ros-pkg:master Jul 16, 2021
@knorth55 knorth55 deleted the fetch-not-use-applist branch July 16, 2021 10:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants