Mavsdk: Trouble using shell plugin

[JonasVautherin]

I was trying to play with the shell plugin. So I connected a pixhawk to my computer (available on serial:///dev/ttyACM0), and ran the following code, which is essentially a modified takeoff_and_land.cpp example (gist here):

#include <chrono>
#include <cstdint>
#include <mavsdk/mavsdk.h>
#include <mavsdk/plugins/shell/shell.h>
#include <iostream>
#include <thread>

using namespace mavsdk;
using namespace std::this_thread;
using namespace std::chrono;

#define ERROR_CONSOLE_TEXT "\033[31m" // Turn text on console red
#define TELEMETRY_CONSOLE_TEXT "\033[34m" // Turn text on console blue
#define NORMAL_CONSOLE_TEXT "\033[0m" // Restore normal console colour

void usage(std::string bin_name)
{
    std::cout << NORMAL_CONSOLE_TEXT << "Usage : " << bin_name << " <connection_url>" << std::endl
              << "Connection URL format should be :" << std::endl
              << " For TCP : tcp://[server_host][:server_port]" << std::endl
              << " For UDP : udp://[bind_host][:bind_port]" << std::endl
              << " For Serial : serial:///path/to/serial/dev[:baudrate]" << std::endl
              << "For example, to connect to the simulator use URL: udp://:14540" << std::endl;
}

int main(int argc, char** argv)
{
    Mavsdk dc;
    std::string connection_url;
    ConnectionResult connection_result;

    bool discovered_system = false;
    if (argc == 2) {
        connection_url = argv[1];
        connection_result = dc.add_any_connection(connection_url);
    } else {
        usage(argv[0]);
        return 1;
    }

    if (connection_result != ConnectionResult::SUCCESS) {
        std::cout << ERROR_CONSOLE_TEXT
                  << "Connection failed: " << connection_result_str(connection_result)
                  << NORMAL_CONSOLE_TEXT << std::endl;
        return 1;
    }

    // We don't need to specify the UUID if it's only one system anyway.
    // If there were multiple, we could specify it with:
    // dc.system(uint64_t uuid);
    System& system = dc.system();

    std::cout << "Waiting to discover system..." << std::endl;
    dc.register_on_discover([&discovered_system](uint64_t uuid) {
        std::cout << "Discovered system with UUID: " << uuid << std::endl;
        discovered_system = true;
    });

    // We usually receive heartbeats at 1Hz, therefore we should find a system after around 2
    // seconds.
    sleep_for(seconds(2));

    if (!discovered_system) {
        std::cout << ERROR_CONSOLE_TEXT << "No system found, exiting." << NORMAL_CONSOLE_TEXT
                  << std::endl;
        return 1;
    }

    Shell shell(system);

    shell.shell_command_response_async([](const Shell::Result result, const Shell::ShellMessage msg) {
        std::cout << "result: " << Shell::result_code_str(result) << std::endl << "msg: " << msg.data << std::endl << std::endl;
    });

    Shell::ShellMessage msg{true, 200, "help"};
    shell.shell_command(msg);

    std::cout << "Finished..." << std::endl;
    while (true) {
        sleep_for(seconds(1));
    }

    return 0;
}

The interesting part woud be:

Shell shell(system);

    shell.shell_command_response_async([](const Shell::Result result, const Shell::ShellMessage msg) {
        std::cout << "result: " << Shell::result_code_str(result) << std::endl << "msg: " << msg.data << std::endl << std::endl;
    });

    Shell::ShellMessage msg{true, 200, "help"};
    shell.shell_command(msg);

The output I get is:

Finished...
result: Success
msg: help

which is unexpected (it seems like it "copies" my message). I tried different ways (sending my message multiple times, adding delays, a thread for the callback), but I never manage to get a useful response.

What am I doing wrong?

[irsdkv]

Hm.
"help" looks ok, this is a cmdline echo.
But also should be an a command response of course.
Now I can't do it myself, please try to increase wait time (2s for example), may be scheduling of cmd response in FMU requires more time.

[irsdkv]

It should be this line:
https://github.com/mavlink/MAVSDK/pull/878/files#diff-f4c5701958a5b01cfeef470e26440de5R115

I see that can confuse, but we can't know how long may be a response. On the other hand if we don't receive termination symbol we can suppose that transfer was not completed.

May be we should add some result state such as "TRANSFER_DOES_NOT_FINISHED"?

I don't implement it yet because of I didn't find some standard specification for MAVLink shell protocol and I can't be sure that all systems sends termination symbol at the end of response

[JonasVautherin]

Now I can't do it myself, please try to increase wait time (2s for example), may be scheduling of cmd response in FMU requires more time.

I tried up to 5 seconds, but I never receive more than "help` in the answer... I would expect a lot of text to come back from that command, right?

May be we should add some result state such as "TRANSFER_DOES_NOT_FINISHED"?

Agreed. I think we want to block until the response is received (or TIMED_OUT). In case another command is sent before the previous answer is received, it should say something like BUSY (equivalent to your TRANSFER_NOT_FINISHED suggestion, but we use BUSY in the other plugins).

Just like my linux terminal: when I send a command, I cannot send another one before I get its result. Does that make sense?

[JonasVautherin]

I don't implement it yet because of I didn't find some standard specification for MAVLink shell protocol and I can't be sure that all systems sends termination symbol at the end of response

Right, that would be a discussion to have in the MAVLink repo/call, probably.

[irsdkv]

Yes, at this time we check if shell is busy
https://github.com/mavlink/MAVSDK/pull/878/files#diff-f4c5701958a5b01cfeef470e26440de5R75

I will do some tests on my setup later and let you know (I remember that on MAVSDK-Python all worked ok with "help" command). I have some assumption about that

[JonasVautherin]

Thanks a lot!

[julianoes]

I tried that example to with the same result. I first tried with help and then commander status:

cmake -Bbuild -H. && cmake --build build -j8 && build/shell serial:///dev/ttyACM0
-- Configuring done
-- Generating done
-- Build files have been written to: /home/julianoes/src/MAVSDK/example/shell/build
[100%] Built target shell
[04:00:13|Info ] MAVSDK version: 0.23.0 (mavsdk_impl.cpp:25)
[04:00:13|Debug] New: System ID: 0 Comp ID: 0 (mavsdk_impl.cpp:401)
Waiting to discover system...
[04:00:13|Debug] Component Autopilot (1) added. (system_impl.cpp:386)
[04:00:13|Debug] Discovered 1 component(s) (UUID: 3689944586785403702) (system_impl.cpp:556)
Discovered system with UUID: 3689944586785403702
[04:00:14|Debug] MAVLink: critical: Data link lost (system_impl.cpp:292)
[04:00:14|Debug] MAVLink: info: Data link regained (system_impl.cpp:292)
Finished...
result: Success
msg: commander status

I then used mavlink_shell.py and it showed me:

Connecting to MAVLINK...

nsh: helphelphelpcommander: command not found

This means the commands are sent but not executed, presumably enter/newline is missing.

[julianoes]

Yes, just add \n and it starts working!

[irsdkv]

Oh, I forget that :man_facepalming: .
Thank you @julianoes !
Anyway I will implement 'NOT_FINISHED'
May be need also add implementation for appending 'n' by default?

[JonasVautherin]

Appending 'n' makes sense to me. But I'm not sure anymore for the NOT_FINISHED. Some messages never finish, according to @julianoes. In which case it is fine as it is now... right?

[irsdkv]

I think that some command can require some time to finish (may be more that 100ms). Or if MAVLink flow overloaded - there also can cause some delays.
User can be confused if, for example,

Shell::ShellMessage msg{true, 50, "help"};

result will be Success, but data field only contain echo.

[julianoes]

If you do a command like top it keeps outputting.

I think automatically appending \n is fine.

[JonasVautherin]

@irsdkv: I'm setting a timeout of 1000 [ms], and it seems to work nicely. What does it bring to have a lower timeout? Doesn't seem like it slows down my interactions with 1 sec...

[irsdkv]

I'm setting a timeout of 1000 [ms], and it seems to work nicely. What does it bring to have a lower timeout? Doesn't seem like it slows down my interactions with 1 sec...

Yes, timeout in this case only set maximum delay between MAVLink response messages.
In case of message with ESC symbol will be received, transfer will consider finished and callback will be called immediately. (see this)

[JonasVautherin]

Got it! All my concerns are addressed, I can close this now. Thanks for the help!