dronecodeのコーディングルールを拝見いたしました。

あまりにもざっくりしていることもあり、保守性や品質まではいき渡っていなさそうと感じています。

そこで、日本には、組み込み系ソフトについて、コーディングルールというのを事細かく定義してるものがあります。

例えば、

・MISRAとか

・IPAの「組込みソフトウェア開発向けコーディング作法ガイド[C++言語版]」や

・凡ミスによる障害の回避ルール

これらのルールをベースにArduCopterのコードの品質を上げていきたい。

また、その過程で、不安なコードがあれば、不安を軽減していきたい。

その情報や修正案などのアップしたいと思い、このディスカッションを立ち上げました。

何卒、よろしくおねがいいたします。

You need to be a member of diydrones to add comments!

Join diydrones

Email me when people reply –

Replies

  • これはわかりやすくてよいですね。

    murata,katsutoshi said:

    ArduPlane / control_modes.cppreadSwitch メソッドの判定条件がもう少しシンプルにできると思います。

    下記のようなスケールで判定値を決定しています。

    -------900(255)-------1230(0)-------1360(1)-------1490(2)-------1620(3)-------1749(4)-------2199(5)-------(255)

    改善前

        if (pulsewidth <= 900 || pulsewidth >= 2200) return 255;            // This is an error condition
        if (pulsewidth > 1230 && pulsewidth <= 1360) return 1;
        if (pulsewidth > 1360 && pulsewidth <= 1490) return 2;
        if (pulsewidth > 1490 && pulsewidth <= 1620) return 3;
        if (pulsewidth > 1620 && pulsewidth <= 1749) return 4;              // Software Manual
        if (pulsewidth >= 1750) return 5;                                                           // Hardware Manual
        return 0;

    改善案

        if (pulsewidth <= 900 || pulsewidth >= 2200) return 255;            // This is an error condition

        if (pulsewidth <= 1230) return 0; ★この条件を入れることでシンプルになります。
        if (pulsewidth <= 1360) return 1;
        if (pulsewidth <= 1490) return 2;
        if (pulsewidth <= 1620) return 3;
        if (pulsewidth <= 1749) return 4;              // Software Manual
        return 5;   

  • 確か、ARMEDが記録されていない限り、ステータスはDISARMED、というのが暗黙のルールだったと思います。


    murata,katsutoshi said:

    APM:Copter V3.4-dev (dbdd86ad) で「DISARMED」状態でログされることを確認しました。


    DISARMED状態であることが「Mechanical Failure」には表示されない様です。

    DISARMEDのログ出力が可となったため、解析情報として「DISARMED」を示す表示必要と感じました。

    Mission Plannerについてもソース理解を進めてみます。

    3702285417?profile=original

    3702283182?profile=original

  • APMrover2capabilities.cppinit_capabilities メソッドの処理について

    同変数の各ビットに設定するために3回呼んでいます。


        hal.util->set_capabilities(MAV_PROTOCOL_CAPABILITY_MISSION_FLOAT);
        hal.util->set_capabilities(MAV_PROTOCOL_CAPABILITY_PARAM_FLOAT);
        hal.util->set_capabilities(MAV_PROTOCOL_CAPABILITY_MISSION_INT);


    ビッドが異なることから1回で可能です。

        hal.util->set_capabilities(MAV_PROTOCOL_CAPABILITY_MISSION_FLOAT | MAV_PROTOCOL_CAPABILITY_PARAM_FLOAT | MAV_PROTOCOL_CAPABILITY_MISSION_INT);

  • ArduPlane / control_modes.cppreadSwitch メソッドの判定条件がもう少しシンプルにできると思います。

    下記のようなスケールで判定値を決定しています。

    -------900(255)-------1230(0)-------1360(1)-------1490(2)-------1620(3)-------1749(4)-------2199(5)-------(255)

    改善前

        if (pulsewidth <= 900 || pulsewidth >= 2200) return 255;            // This is an error condition
        if (pulsewidth > 1230 && pulsewidth <= 1360) return 1;
        if (pulsewidth > 1360 && pulsewidth <= 1490) return 2;
        if (pulsewidth > 1490 && pulsewidth <= 1620) return 3;
        if (pulsewidth > 1620 && pulsewidth <= 1749) return 4;              // Software Manual
        if (pulsewidth >= 1750) return 5;                                                           // Hardware Manual
        return 0;

    改善案

        if (pulsewidth <= 900 || pulsewidth >= 2200) return 255;            // This is an error condition

        if (pulsewidth <= 1230) return 0; ★この条件を入れることでシンプルになります。
        if (pulsewidth <= 1360) return 1;
        if (pulsewidth <= 1490) return 2;
        if (pulsewidth <= 1620) return 3;
        if (pulsewidth <= 1749) return 4;              // Software Manual
        return 5;   

  • 下記、改善案をPRさせていただき、採用いただきました。

            if (c == 'c') {
                break;

            } else if (c == 0x03 || c == 'q') {
    murata,katsutoshi said:

    setup.cppesc_calib メソッドで、入力文字コードですでに「'c'」をチェックしており、0x63でチェックする意は?

    不要なチェックと思うんだけど。

            if (c == 'c') {
                break;

            } else if (c == 0x03 || c == 0x63 || c == 'q') {

  • Navio2 + Raspberry Pi 3でArdupilotをGDBでデバッグする手順を記します。

    GDBでデバッグできるのは、組み込み開発には必須です。

    デバシリはあくまで保守用なので。

    デバッグオプションを指定します。

    waf configure --board=navio2 --debug

    クリーン

    waf clean --targets bin/arducopter-hexa

    デバッグオプションが有効かを確認

    waf -v --targets bin/arducopter-hexa

    クリーン

    waf clean --targets bin/arducopter-hexa

    ビルド

    waf --targets bin/arducopter-hexa

    cd ./build/navio2-debug/bin

    sudo ./arducopter-hexa -A udp:192.168.0.15:14550

    Raspberry Pi 2/3 with BCM2709!

    デバッグ開始

    ps -axでプロセスIDを確認

    sudo gdb

    attach プロセスID

    b ArduCopter.cpp:474

    c

    3702283535?profile=original

  • APM:Copter V3.4-dev (dbdd86ad) で「DISARMED」状態でログされることを確認しました。


    DISARMED状態であることが「Mechanical Failure」には表示されない様です。

    DISARMEDのログ出力が可となったため、解析情報として「DISARMED」を示す表示必要と感じました。

    Mission Plannerについてもソース理解を進めてみます。

    3702283216?profile=original

    3702283182?profile=original

  • せんざい様、TOMI様

    コメントありがとうございます。

    機体、修理中なので、修理がおわりましたら確認してみます。

    Goro Senzai said:

    ありがとうございます。

    確かにLOG_DISARMEDの独立パラメーター化はmasterにしか反映されていないので、Copterの現行バージョンを使う場合は、ご指摘のようにビットマスクを使うのがが正解です。


    Tomi Mizya said:

    お望みの機能はコレですかね?

    3702278886?profile=original

  • せんざい様

    わかりやすく、ご説明いただきありがとうございます。

    非飛行状態における異常検出について、条件を加えることで対処できるかもしれません。

    確認してみます。


    Goro Senzai said:

    Bad AHRSが頻出する件は、3.3.2でフィルターが入って治ったはずですが、長時間平坦でないところに置いておくとIタームが蓄積してエラーになるのかもしれません。

    AHRSのエラーは、加速度計のキャリブレーションをやり直すことで解決することが多いですが、キャリブレーション自体、水平なデスク、垂直な壁面、水平器等を使って完全に静止した状態で行っても、温度変化でセンサー出力がドリフトするので、高価なセンサー機器がやっているように、温度変化や他のセンサーの値をモニタリングしながらアクティブに補正を行っていく仕組みを実装しないと安定した動作は難しいのではないかと思います。

    murata,katsutoshi said:

    コメントありがとうございます。

    AC 3.3.3、3.4.0-rc1 には組み込まれていないようで、残念です。

    APは3.6.0で組み込んだようです。

    Goro Senzai said:

    LOG_DISARMED=1 でARMする前からログが記録されると思います。


    murata,katsutoshi said:

    室内で、電源を入れただけの状態「DISARMED」で、30分以上、放置していたらmission plannerの画面に「Bad AHRS」が表示されました。

    再度、実施したところ、同じ30分ごろに、同メッセージが表示しました。

    この現象は、すでに、DIYDronesで上がっていました。

    AC3.3.3ではまだ解消していないような感じです。

    ログ解析してみようとしたところ、ログファイルが作成されていませんでした。

    電源を入れただけだとログファイルを作成するようになっていないのか、まだ、ソースの確認していませんが。。。

    初期動作異常を解析するためにもログは必要と思う。

    ログファイル作成のタイミングを調査します。

    3702278766?profile=original

  • Bad AHRSが頻出する件は、3.3.2でフィルターが入って治ったはずですが、長時間平坦でないところに置いておくとIタームが蓄積してエラーになるのかもしれません。

    AHRSのエラーは、加速度計のキャリブレーションをやり直すことで解決することが多いですが、キャリブレーション自体、水平なデスク、垂直な壁面、水平器等を使って完全に静止した状態で行っても、温度変化でセンサー出力がドリフトするので、高価なセンサー機器がやっているように、温度変化や他のセンサーの値をモニタリングしながらアクティブに補正を行っていく仕組みを実装しないと安定した動作は難しいのではないかと思います。

    murata,katsutoshi said:

    コメントありがとうございます。

    AC 3.3.3、3.4.0-rc1 には組み込まれていないようで、残念です。

    APは3.6.0で組み込んだようです。

    Goro Senzai said:

    LOG_DISARMED=1 でARMする前からログが記録されると思います。


    murata,katsutoshi said:

    室内で、電源を入れただけの状態「DISARMED」で、30分以上、放置していたらmission plannerの画面に「Bad AHRS」が表示されました。

    再度、実施したところ、同じ30分ごろに、同メッセージが表示しました。

    この現象は、すでに、DIYDronesで上がっていました。

    AC3.3.3ではまだ解消していないような感じです。

    ログ解析してみようとしたところ、ログファイルが作成されていませんでした。

    電源を入れただけだとログファイルを作成するようになっていないのか、まだ、ソースの確認していませんが。。。

    初期動作異常を解析するためにもログは必要と思う。

    ログファイル作成のタイミングを調査します。

    3702278766?profile=original

This reply was deleted.