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

Bash scripting for posix #5162

Closed
wants to merge 92 commits into from
Closed

Bash scripting for posix #5162

wants to merge 92 commits into from

Conversation

julianoes
Copy link
Contributor

@julianoes julianoes commented Jul 29, 2016

tl;dr

This adds bash scripting for POSIX. The startup file is now evaluated by bash.
Additionally, PX4 commands can be sent from other terminals, allowing commands such as px4-commander status or px4-listener sensor_combined from anywhere on the machine.

Description

This PR adds the possibility to spawn PX4 modules out of bash. Basically,
the main executable can now be started as a server/daemon or as a
client.
The server replaces the existing functionality of the main exe with
the pxh shell, however, it also opens a pipe that clients can talk to.

Clients can run or spawn PX4 modules or commands by connecting to the
server over the pipe. They clients will get the stdout and return value
of their commands via a client specific pipe back.

The main exe starts as a server and runs a startup script using bash. Each command in the startup script such as 'commander start' gets aliased with a prefix as 'px4-commander start' and symlinked to the px4 main executable.
If the main executable is called with the prefix 'px4-' which is passed in argv[0], it will act as a client and therefore send the command 'commander start' to the server. Once this command has been run in the server, the client will receive the return value and the bash script will continue with the next command.

Additionally, this allows commands to be sent from outside pxh, so from other terminals (on the same computer). All that is needed is to have the px4 exe and the symlinks in the PATH.

Thanks to @bkueng for the big help.

Let me know what you think: @jgoppert, @LorenzMeier, @dagar, @mcharleb

Adresses: #4800

Progress to get this PR in

  • Main SITL startup transferred to unified rcS startup script
  • Additional SITL startups into rcS
  • Check CI on jenkins
  • Script Snapdragon startup
  • Script Bebop startup
  • Script Raspberry Pi startup
  • Upload mixers
  • Stress test whole system with some scripts
  • Allow listener to keep looping
  • Add Devguide documentation

Future work after this PR

  • Unify NuttX and Posix startup

How to test

SITL

The normal SITL startup already uses the new unified SITL startup script rcS, it can get started with:

make posix gazebo
make posix jmavsim

To send a command in another terminal window, adapt the PATH:

export PATH=$PATH:~/src/Firmware/build_posix_sitl_default/bin

and send a command:

px4-commander status

Or listen to a uORB message:

px4-listener vehicle_attitude

Press Ctrl+C to exit.

Snapdragon

To test this on Snappy, just build and upload normally:

make eagle_default upload

Open adb shell, and start it with:

cd /home/linaro/bin
export PATH=$PATH:/home/linaro/bin
./px4 rcS

Then choose, the airframe config using the param SYS_AUTOSTART and restart. The current options are:

SYS_AUTOSTART 4100: generic (flight) Quad
SYS_AUTOSTART 4200: Microheli 200QX

@dagar
Copy link
Member

dagar commented Jul 29, 2016

Very cool. For startup scripts I wonder if there's a subset of bash and nsh we could safely use and then completely unify startup.

@julianoes
Copy link
Contributor Author

For startup scripts I wonder if there's a subset of bash and nsh we could safely use and then completely unify startup.

Yer that would be the plan. Most commands of the nsh are already in bash, however, I found set which is "missing".

@bharath374
Copy link
Contributor

Julian, this is a great solution!

One comment: The following lines should stay under "if param compare SYS_AUTOSTART 4100" since they're not been tested in other configurations.

qshell gps start -d /dev/tty-4
qshell df_hmc5883_wrapper start
qshell df_trone_wrapper start

@julianoes julianoes force-pushed the feature_shell branch 2 times, most recently from 4b60fd8 to c66315a Compare August 3, 2016 05:58
@@ -160,6 +177,34 @@ endforeach()

install(TARGETS px4 DESTINATION bin)
install(DIRECTORY ${PROJECT_SOURCE_DIR}/ROMFS DESTINATION ${CMAKE_INSTALL_DATADIR}/${PROJECT_NAME})
install(DIRECTORY ${PROJECT_SOURCE_DIR}/posix-configs DESTINATION ${CMAKE_INSTALL_DATADIR}/${PROJECT_NAME})
install(DIRECTORY ${PROJECT_SOURCE_DIR}/posix-configs/SITL/init DESTINATION ${CMAKE_BINARY_DIR}/rootfs/etc/init)
Copy link
Member

Choose a reason for hiding this comment

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

Is this correct? CMAKE_BINARY_DIR seems wrong

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm gonna remove the install stuff again, not sure how this should work.

Copy link
Member

Choose a reason for hiding this comment

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

@julianoes I think it would be worthwhile to rebase on #5255 now.

@bkueng
Copy link
Member

bkueng commented Aug 3, 2016

Finished reviewing, added some small obvious changes. Comments adress mostly small stuff, I think the cmake needs some more work. But overall really good code!

@julianoes
Copy link
Contributor Author

@bkueng Thanks for the review!

@bkueng bkueng mentioned this pull request Aug 4, 2016
5 tasks
@lucasdemarchi
Copy link
Contributor

I think I'm probably late for this, but I was working on the current state of master branch and then I saw this PR. Quick question: why do you want to start the px4 components via arbitrary shell scripts? Wouldn't a declarative approach via configuration file (ini, json or other format) and then probably a single binary to control the internal parts be simpler?

@LorenzMeier
Copy link
Member

Because it allows you to follow the UNIX system design of a collection of small apps and mixing calls to the system with calls to the PX4 middleware.

It's easy to support a JSON file format as well, but that would be rather for fixed vehicle configurations (which is important for products).

I suggest you try this PR when it's done - running the uORB listener in a separate she'll and watching different topics in parallel is an extremely powerful development tool.

@lucasdemarchi
Copy link
Contributor

Because it allows you to follow the UNIX system design of a collection of small apps and mixing calls to the system with calls to the PX4 middleware.

It's certainly an improvement over the current one. However since the wrappers are actually talking to another daemon (the px4 binary) that responds accordingly, it's more a client/server approach than a classical "collection of small apps". In this case I usually see people using a declarative approach, i.e. a config file, because it avoids tempting hacks to add lots of sleeps here and there, scripting all the startup sequence. It's more or less a similar scenario of init implementation in which everybody went away from pure script init (be it systemd, upstart, android init, MacOS, etc) to a more declarative approach, even if in some of them still exists limited support for scripting.

@julianoes
Copy link
Contributor Author

Ok, I've rebased this once again. I was able to do make posix jmavsim and get it to takeoff. Most of the rest is presumably broken (again) though.

@dagar
Copy link
Member

dagar commented Apr 26, 2017

@julianoes I'm really looking forward to this. What's needed to get it in? The PR is massive, are there smaller pieces we could start breaking off and pulling in?

@julianoes
Copy link
Contributor Author

@dagar: no, I think we need to consume it in one piece. Next steps is porting all the startup scripts into one. Let me plan this for this weekend.

@TSC21
Copy link
Member

TSC21 commented Aug 20, 2017

@julianoes do you have the required bandwidth to proceed with this. This would be a very important update for our interface with the flight controller.

@TSC21 TSC21 closed this Aug 20, 2017
@TSC21 TSC21 reopened this Aug 20, 2017
@LorenzMeier
Copy link
Member

@julianoes I'm closing this as stale. I would love to see this in by all means. But I think it would be good if you could break it down instead into one PR that adds the capability (but leaves the current startup scripts) and then iterate on breaking those down.

That should be much quicker and take much less of your time than re-architecting the complete startup of all targets.

Thanks!

@julianoes
Copy link
Contributor Author

Agreed. I need a week of holiday for this one.

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.

None yet

10 participants