Skip to content

Remove source alias#15970

Merged
dagar merged 3 commits into
masterfrom
master-pr-source
Oct 15, 2020
Merged

Remove source alias#15970
dagar merged 3 commits into
masterfrom
master-pr-source

Conversation

@davids5
Copy link
Copy Markdown
Member

@davids5 davids5 commented Oct 14, 2020

Per today's dev call and @bkueng #15139 (comment)

Use source for . and a prefix of ${R} set to / for nuttx and ${pwd} for linux

@davids5 davids5 requested review from MaEtUgR and bkueng October 14, 2020 20:21
@davids5
Copy link
Copy Markdown
Member Author

davids5 commented Oct 14, 2020

ehh

Now we get

etc/init.d-posix/rcS: 81: /home/david_s5/src/PX4/repos/mainline/Firmware/build/px4_sitl_default/tmp/rootfs/etc/init.d/rc.vehicle_setup: source: not found
etc/init.d-posix/rcS: 84: /home/david_s5/src/PX4/repos/mainline/Firmware/build/px4_sitl_default/tmp/rootfs/etc/init.d/rc.vehicle_setup: source: not found

NuttX is respecting . or source.

From what I can tell googling "In the POSIX standard, which /bin/sh is supposed to respect, the command is . (a single dot), not source"

Any Ideas?

@davids5 davids5 requested a review from dagar October 14, 2020 21:31
Comment thread platforms/posix/src/px4/common/px4-alias.sh_in Outdated
Copy link
Copy Markdown
Member

@MaEtUgR MaEtUgR left a comment

Choose a reason for hiding this comment

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

Thanks for looking into this @davids5 ! I quickly checked and SITL works as expected. I can quickly check if Beat's alternative would work as well.

Comment thread ROMFS/cannode/init.d/rcS Outdated
@MaEtUgR
Copy link
Copy Markdown
Member

MaEtUgR commented Oct 15, 2020

That works as well in SITL: https://github.com/PX4/Firmware/compare/pr-source-alternative-test

And on Nuttx set sh . if that works.

Not sure where to adjust it for NuttX

@davids5
Copy link
Copy Markdown
Member Author

davids5 commented Oct 15, 2020

@MaEtUgR @bkueng - thank you for testing and the suggestions.

That works as well in SITL: https://github.com/PX4/Firmware/compare/pr-source-alternative-test

And on Nuttx set sh . if that works.

Not sure where to adjust it for NuttX

Given the the POSIX comments, I have gone back to using '.' everywhere with

NuttX: R=/
Linux R="`pwd`/"

I am in favor of this because it shows the correct intent: "sourc-ing" and "exec-ing" are not the same.

sh path_to_script - launch a process running path_to_script which can not change parent environment.
. path_to_script and source path_to_script include the path_to_script which can change parent environment.

Let's see what breaks now :)

We should be able to dump the sh alias as well. But I will wait for your testing

@MaEtUgR
Copy link
Copy Markdown
Member

MaEtUgR commented Oct 15, 2020

I did another test. Works great on arch
image

@MaEtUgR
Copy link
Copy Markdown
Member

MaEtUgR commented Oct 15, 2020

We should be able to dump the sh alias as well. But I will wait for your testing

I also tried removing it in the SITL aliases and didn't see any error.

Output

[0/1] Re-running CMake...
-- PX4 version: v1.11.0-rc3-488-g891c6e24b1
-- PX4 config file: /home/maetugr/Firmware/boards/px4/sitl/default.cmake
-- PX4 config: px4_sitl_default
-- PX4 platform: posix
-- PX4 lockstep: enabled
-- cmake build type: RelWithDebInfo
-- Building for code coverage
-- ccache enabled (export CCACHE_DISABLE=1 to disable)
-- build type is RelWithDebInfo
-- PX4 ECL: Very lightweight Estimation & Control Library v1.9.0-rc1-456-gf666ebb
-- ROMFS: px4fmu_common
-- Configuring done
-- Generating done
-- Build files have been written to: /home/maetugr/Firmware/build/px4_sitl_default
[0/1] cd /home/maetugr/Firmware/build/px4_sitl_default/tmp && /home/maetugr/Firmware/Tools/sitl_run.sh /home/maetugr/Firmware/build/px4_sitl_default/bin/px4 none jmavsim none none /home/maetugr/Firmware /home/maetugr/Firmware/build/px4_sitl_default
SITL ARGS
sitl_bin: /home/maetugr/Firmware/build/px4_sitl_default/bin/px4
debugger: none
program: jmavsim
model: none
world: none
src_path: /home/maetugr/Firmware
build_path: /home/maetugr/Firmware/build/px4_sitl_default
empty model, setting iris as default
SITL COMMAND: "/home/maetugr/Firmware/build/px4_sitl_default/bin/px4" "/home/maetugr/Firmware/build/px4_sitl_default"/etc -s etc/init.d-posix/rcS -t "/home/maetugr/Firmware"/test_data
INFO  [px4] Creating symlink /home/maetugr/Firmware/build/px4_sitl_default/etc -> /home/maetugr/Firmware/build/px4_sitl_default/tmp/rootfs/etc

______  __   __    ___ 
| ___ \ \ \ / /   /   |
| |_/ /  \ V /   / /| |
|  __/   /   \  / /_| |
| |     / /^\ \ \___  |
\_|     \/   \/     |_/

px4 starting.

INFO  [px4] Calling startup script: /bin/sh etc/init.d-posix/rcS 0
Info: found model autostart file as SYS_AUTOSTART=10016
INFO  [param] selected parameter default file eeprom/parameters_10016
[param] Loaded: eeprom/parameters_10016
INFO  [dataman] Unknown restart, data manager file './dataman' size is 11798680 bytes
INFO  [simulator] Waiting for simulator to accept connection on TCP port 4560
Buildfile: /home/maetugr/Firmware/Tools/jMAVSim/build.xml

make_dirs:

compile:

create_run_jar:

copy_res:

BUILD SUCCESSFUL
Total time: 0 seconds
Options parsed, starting Sim.
Starting GUI...
INFO  [simulator] Simulator connected on TCP port 4560.
Init MAVLink
INFO  [commander] LED: open /dev/led0 failed (22)
INFO  [init] Mixer: etc/mixers/quad_w.main.mix on /dev/pwm_output0
INFO  [mavlink] mode: Normal, data rate: 4000000 B/s on udp port 18570 remote port 14550
INFO  [mavlink] mode: Onboard, data rate: 4000000 B/s on udp port 14580 remote port 14540
INFO  [mavlink] mode: Onboard, data rate: 4000 B/s on udp port 14280 remote port 14030
INFO  [logger] logger started (mode=all)
INFO  [logger] Start file log (type: full)
INFO  [logger] [logger] ./log/2020-10-15/14_08_09.ulg
INFO  [logger] Opened full log file: ./log/2020-10-15/14_08_09.ulg
INFO  [mavlink] MAVLink only on localhost (set param MAV_BROADCAST = 1 to enable network)
INFO  [px4] Startup script returned successfully
pxh> INFO  [mavlink] partner IP: 127.0.0.1
INFO  [ecl/EKF] 2564000: reset position to last known position
INFO  [ecl/EKF] 2564000: reset velocity to zero
INFO  [ecl/EKF] 8664000: EKF aligned, (baro hgt, IMU buf: 18, OBS buf: 14)
INFO  [ecl/EKF] 9884000: GPS checks passed

pxh> 
pxh> 
pxh> commINFO  [ecl/EKF] 14416000: reset position to GPS
INFO  [ecl/EKF] 14416000: reset velocity to GPS
INFO  [ecl/EKF] 14416000: starting GPS fusion
pxh> commanderINFO  [tone_alarm] home set
ERROR [tone_alarm] notify negative
pxh> commander takeoff
pxh> INFO  [commander] Armed by internal command
INFO  [navigator] Using minimum takeoff altitude: 2.50 m
INFO  [commander] Takeoff detected

PX4 Exiting...
pxh> Exiting NOW.
ninja: build stopped: interrupted by user.
make: *** [Makefile:226: px4_sitl] Interrupt

@davids5
Copy link
Copy Markdown
Member Author

davids5 commented Oct 15, 2020

We should be able to dump the sh alias as well. But I will wait for your testing

I also tried removing it in the SITL aliases and didn't see any err

@MaEtUgR - OK removed the sh alias as well.

@dagar once this passes CI it should be ready to merge.

@dagar dagar merged commit d4ae398 into master Oct 15, 2020
@dagar dagar deleted the master-pr-source branch October 15, 2020 21:11
@MaEtUgR
Copy link
Copy Markdown
Member

MaEtUgR commented Oct 19, 2020

SITL works as expected now on Arch and Cygwin. Thanks! NuttX fix on Cygwin still pending. I'll have another look at it.

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.

4 participants