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: Save new/changed behavior on the robot through SSH #21

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

feat: Save new/changed behavior on the robot through SSH #21

wants to merge 1 commit into from

Conversation

alireza-hosseini
Copy link

@alireza-hosseini alireza-hosseini commented Jul 9, 2018

What I did:

  • Used scp2 and ssh2 node modules to update the behavior package on the robot according to new changes made on this package (adding new behavior, modifying an existing one) through SSH.

  • The SSH configurations (hostname and username) shall be set in new input boxes provided in Code Generation section of Configuration tab. This feature is disabled by default and it can be set to enabled from the same section.
    screenshot from 2018-06-03 13-27-16

  • For security reasons (to don't store it as a cookie) password will be prompted on clicking the save behavior button.
    image

  • Refactored the package.json and added dependency to scp2 module.

Important notes:

  • The behavior package shall be already available on the robot.
  • In order to be able to find package the path, the ROS environment variables shall be set properly on .profile script.
  • In order to use this new feature its dependencies needs to be installed. To do so, the npm install command will be automatically executed during cmake.
  • If you saw any indentation incossitency, its because of using tabs instead of spaces in the whole original source code, while I used 4-spaces. This shall be fixed in a separate PR.

Feature works:

  • Unfortunately, javascript doesn't provide a prompt box with password masking feature, as a matter of the fact, the current password prompt is unmasked. A proper solution is to replace the usage of the password with SSH keys.
  • Currently, the whole package will be overwritten by clicking on save behavior. This can be improved by just updating the modified files.

@alireza-hosseini alireza-hosseini changed the title feat: Save new/changed behavior on the robot through SSH (#1) feat: Save new/changed behavior on the robot through SSH Jul 9, 2018
@alireza-hosseini
Copy link
Author

@pschillinger Any comment on this?

@pschillinger
Copy link
Member

This sounds like an exciting feature and I think it fits perfectly into the concept of the FlexBE app! Thanks a lot for your effort!

I wanted to get #19 out of the way, then I will try it out more carefully.

@utkarsh447
Copy link

Hi @alireza-hosseini , Thanks for adding the ssh feature in flexBE App.
I am getting an issue in implementing it. My flexbe_app is running on one system and flexbe_onboard is running on another system. I am able to run the behavior but having trouble in editing the code via ssh.

On your modifications, I have entered Robot's hostname as it's IP and Robot's username in Configuration -> Code Generation Tab, And on clicking save behaviour, it gives "Permission Denied Error".
Please refer the screenshot below,
7

On running it in super user,
10

And my .profile contains this code:

# ~/.profile: executed by the command interpreter for login shells.
# This file is not read by bash(1), if ~/.bash_profile or ~/.bash_login
# exists.
# see /usr/share/doc/bash/examples/startup-files for examples.
# the files are located in the bash-doc package.

# the default umask is set in /etc/profile; for setting the umask
# for ssh logins, install and configure the libpam-umask package.
#umask 022

# if running bash
if [ -n "$BASH_VERSION" ]; then
    # include .bashrc if it exists
    if [ -f "$HOME/.bashrc" ]; then
	. "$HOME/.bashrc"
    fi
fi

# set PATH so it includes user's private bin directories
PATH="$HOME/bin:$HOME/.local/bin:$PATH"

Please suggest the changes to be done to make files edit and make it work?

@alireza-hosseini
Copy link
Author

@utkarsh447 Thanks for trying my proposed feature,
Regarding the first error I think it is related to the fact that probably when you tried to save the behavior, it was actually running on the remote machine so that "example_behavior_sm.py" was in read-only state which means scp cannot overwrite the changes on this file.
About the second error, please make sure that you have the following line in your .profile:
source /STATIC_PATH_TO_YOUR_CATKIN_WS/devel/setup.bash

@utkarsh447
Copy link

@utkarsh447 Thanks for trying my proposed feature,
Regarding the first error I think it is related to the fact that probably when you tried to save the behavior, it was actually running on the remote machine so that "example_behavior_sm.py" was in read-only state which means scp cannot overwrite the changes on this file.
About the second error, please make sure that you have the following line in your .profile:
source /STATIC_PATH_TO_YOUR_CATKIN_WS/devel/setup.bash

Thank You very much, It is working as it should!
And your added feature is awesome and really helped me alot..!! :)

* feat: Add input boxes for setting SSH configs
 - These settings will be used for updating the behavior on the robot
 according to new changes made on the behavior using flexbe app.

* feat: Update the behavior pkg on the robot on saveBehavior clicked
 - It will create a SSH tunnel to the robot and overwrite the behavior package

* build: Major refactoring on package.json
 - Add dependency to `scp2`

* Revert "style: Replace tabs with 4space"

* fix: remove spaces only from package path

* build: Install node modules from CMakeLists.txt
@alireza-hosseini
Copy link
Author

@utkarsh447 Cool, glad to hear that ;-)

@pschillinger
Copy link
Member

Thank you for testing @utkarsh447 and thank you again for contributing @alireza-hosseini !

I still have some questions, could you help me with those? I will annotate some in the code for easier reference.

Also, did you test this in combination with making runtime modifications? To me, it looks like these two features would interfere when doing an SSH save while a behavior is running/locked.

I'm really looking forward to merge this feature! I just want to make sure that nothing breaks, of course.

@@ -68,3 +68,5 @@ if(CATKIN_ENABLE_TESTING)
find_package(rostest REQUIRED)
add_rostest(launch/test_report.test)
endif()

execute_process(COMMAND npm --prefix ${PROJECT_SOURCE_DIR} install)
Copy link
Member

Choose a reason for hiding this comment

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

This fails if npm is not installed. Do you see a way to automatically handle npm as a dependency?

{
try
{
var SSHClient = require('ssh2').Client;
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't ssh2 be an npm dependency as well? Even if included in scp2, it is referenced here directly.

username: ssh_username,
password: ssh_password
});
}
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure if I understand the above code block correctly. To me it looks like there is an ssh connection created first and then, within this connection, another scp connection is opened to transfer the file. Do I get it wrong? Or what is the reason for this approach?

ssh_username,
ssh_password,
ssh_hostname,
ssh_package_path);
Copy link
Member

Choose a reason for hiding this comment

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

This contains the password in plain text, right? Is there a way to encrypt? I have no experience with the scp2 package, but do you know how the query string would be further used?

fmessmer added a commit to fmessmer/flexbe_app that referenced this pull request Sep 21, 2023
Change `version` to a Development Tag
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