Px4-autopilot: nuttx vs posix px4_task_spawn_cmd argc and argv

Created on 29 May 2019  路  12Comments  路  Source: PX4/PX4-Autopilot

tl;dr:
px4_task_spawn_cmd sets the argc and argv variables differently on Nuttx and Posix. Nuttx is correct and includes the command name in argv[0]. But the Posix-Implementation only adds the arguments to argv, which is not, what's commonly used.
It's the same on macOS and Linux.

Describe the bug
I took the hello example application as a staring point for my application. Then I noticed, that the argc variable is not the same on nuttx and posix inside a newly spawned task (with px4_task_spawn_cmd).

The task is spawned like this:

px4_task_spawn_cmd("hello",
                   SCHED_DEFAULT, SCHED_PRIORITY_MAX - 5, 2000,
                   PX4_MAIN,
                   (argv) ? (char *const *)&argv[2] : (char *const *)nullptr);

When running hello start, nuttx will set argc and argv to these values inside the task:

  • argc = 1
  • argv[0] = hello

On posix on the other hand:

  • argc = 0
  • argv[0] is empty

When running hello start -h, nuttx will set argc and argv to these values inside the task:

  • argc = 2
  • argv[0] = hello
  • argv[1] = -h

On posix on the other hand:

  • argc = 1
  • argv[0] = -h
  • argv[1] is empty

To Reproduce
Steps to reproduce the behavior:

  1. Take the hello example app
  2. Add some code to output the argc and argv variables
  3. Run the application with different amounts of arguments in Nuttx and Posix
  4. See error

Expected behavior
The argument order and count would be the same on Nuttx and Posix.

bug stale

All 12 comments

Thanks for the issue. This sounds familiar. I always believed that NuttX had it wrong and we needed to work around that. @LorenzMeier can you comment?

I was under the impression that this was an expected behavior from Nuttx. Nuttx task_create adds the task name as the first argument.

I thought this is why run_trampoline in ModuleBase class has the following code.
`#ifdef __PX4_NUTTX
argc -= 1;
argv += 1;

endif`

But I admit, this has bothered me for some time and I had to take extra precaution in my code for arg parsing for Nuttx and posix

Yes, we worked around non-standard behaviour in NuttX (which might have changed by now). Please also keep in mind that we're internally not storing the task name for the PX4 app classes, as the idea is that the arguments are a list of the arguments and not the POSIX interpretation of it. But it should behave the same on all platforms and it looks like it needs a closer look. Something is definitely off.

Just to be clear: The argc and argv are consistent in both NuttX and POSIX and the same as "standard" POSIX (application name as first argument), when an application's main function is called. Just the task creation in POSIX does not fit all of the other argument lists.
Therefore I would recommend to change the SITL/ POSIX implementation, because it's the only part, that does not match the other ones. The issue it, that any change will beak some applications, but I think, that breaking SITL is better than breaking code for the FMU.
This would be easier, even if Lorenz said, that the initial design was meant to be something different.

I can confirm that the behavior is different:

NuttX:

nsh> hello start -h 400 -f 200

argc: 5
0: hello
1: -h
2: 400
3: -f
4: 200

Posix:

pxh> hello start -h 400 -f 200

argc: 4
0: -h
1: 400
2: -f
3: 200

From what I found we would have to add:

 #ifdef __PX4_NUTTX argc -= 1; argv += 1; #endif

for consistency in all _main() functions where we are using argc, argv. Therefore I tend to agree with @jbeyerstedt that we should just use the POSIX convention for SITL as well.

This issue has been automatically marked as stale because it has not had recent activity. Thank you for your contributions.

still a bug (even with an open pull request)

This issue has been automatically marked as stale because it has not had recent activity. Thank you for your contributions.

@julianoes Should this still be open, because you PR is also still open?

Yes, still on my todo list :confused:

This issue has been automatically marked as stale because it has not had recent activity. Thank you for your contributions.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

prothen picture prothen  路  5Comments

zhanghouxin07 picture zhanghouxin07  路  5Comments

julianoes picture julianoes  路  3Comments

julianoes picture julianoes  路  3Comments

ghost picture ghost  路  4Comments