[Add]chapter7 test_opencv package

[MoriKen254]

@RyodoTanaka さん お久しぶりです！お元気にしてますか？ ^^　産総研楽しいですか？久々に話したいですねー。

さて、@RyodoTanaka さんが7章のコードのメンテを依頼されたという件を @TakeshiNishida 先生よりお聞きしましたので、ぜひぜひ本件のプルリクレビューをお願いします！^^

とりあえず、ROS本 7章の一番最初のサンプルだけ動かしました。

ちなみに、本のサンプルコードのコピペでは動きません。以下について修正して、catkin_makeを通し、画像表示まで確認できたものをプルリクしています。

• OpenCVのincludeディレクトリに不備がある問題を修正。
• CMakeListsの設定不備を修正。

一番最初のサンプルでこれなので、以降のサンプルコードがコピペで動くのか、すごーく怪しい気がしてなりません…。

@TakeshiNishida このように、本文のサンプルコードがビルドできないことが確認済みです。 7章はサンプルコードの差し替えを念頭に、最終校正を行う必要があります。サンプルコードのビルドが通らないというのは、プログラミングの解説本として致命的な問題です。必要に応じて、本文の修正も少しは必要かもしれません。

取り急ぎ、以上です。

[MoriKen254]

あ、一応環境を補足。

Ubuntu 16.04, ROS Kineticで動作確認済みです。

[RyodoTanaka]

@MoriKen254 お久しぶりです． 産総研楽しくやってます！ 僕も是非お話したいです． 東京にいらっしゃる際は是非教えてください！

さて，マージについてですが industrial_ciの使い方も覚えたので，そちらでテストが完了してからマージしたいと考えています． 作業時間的に，今日明日で（7/8）までに一通り終わらせる予定です． よろしくお願いします．

[MoriKen254]

@RyodoTanaka 東京行きたいですねー。行けるかなー笑。行くときはお知らせします。

さて、諸々承知しました。 @RyodoTanaka さんであれば不要と思いますが、万が一作業分担が必要であれば、遠慮なく私にもふって下さい。 明日明後日であれば、夜間でよければ手を動かせます。

industrial_ci もお分かりになったということで、心強いです。頼りにさせてくださいね。

よろしくお願致します。

[RyodoTanaka]

@MoriKen254 承知しました． ありがとうございます！

取り急ぎ，本PRの下にindustrial_ciでチェックするブランチを切ってマージ作業を行っていこうと思います． よろしくお願いします．

[TakeshiNishida]

ここに書いていいかわからないんですが、3章のpackage.xmlの12行目と15行目の「mutiple」を「multiple」に修正してもらえませんでしょうか。

7章の修正はどんな感じでしょう。あまり修正が大きくなければいいんですが。

[MoriKen254]

chapter3のpackage.xmlは、ソースコード側では大丈夫でした。

また、修正範囲を見積もるためには、コード全体を見る必要があります。実際にビルドをかけてみないとわからない部分があるからです。 できるだけ本文に大きな変更を加えないで済むようにします。

ざっくりした見積もりですが、以降のコードについてもインクルードの問題だけであると仮定すれば、多分ソースコード側のちょっとした修正と、序盤のCMakeLists.txtの設定周りの文章をちょちょいと変える程度で、大丈夫ではないかと思っています。

取り急ぎビルドできたものからどんどんpushしていきます。コンフリクトしないよう、これから着手するソースコード名を一旦ここで宣言してから、随時pushしていくことにしましょう。

私は今から、test_get_input.cppをやります。

なお、industrial_ciの適用は優先度「低」です。理由は、本文に影響がないからです。

[MoriKen254]

test_get_input.cpp 終わりました。

test_edge_detection.cpp に移ります。

なお、test_get_input.cppも、コピペでは動きませんでした。

• インクルードファイルの修正。
• 主要な処理対象画像が、cv::Matで宣言されていない。
• 変数の命名がCamel型だったので、snake型に修正。
• 全く不要な記載があった行を削除。

（まだ２つ目の段階で恐れ多いのですが、とてもローカルで一度catkin_makeを試したコードとは思えません…。）

なお、別issueで上がっていた、コード規約の方の進捗はどうなっていますか？

[MoriKen254]

test_edge_detection.cpp 完了。

test_feature_detection.cpp 開始。

[MoriKen254]

test_feature_detection.cpp 完了です。

ただ、私がもしやそんなことはないよねぇ？…と、一番懸念したいたことが、ビンゴしてしまいました。

現行のOpenCVではビルド不可

これまでのコードは、ヘッダ修正や文法の問題を解決するといった小手先修正でビルドを通すことができましたが、ここに来て致命的な問題に遭遇しました。

test_feature_detection.cpp では、その程度の修正ではビルドが通りません。使用されているクラス「MserFeatureDetector」が現行のOpenCVでは存在しないからです。

もちろん、古いOpenCVと共存されればビルドは通せるでしょうが、それならその方法を明記するべきですし、そもそもそんなROSの環境を汚しかねないリスクを犯してまでそこに固執する理由はありません。

ROS本なのですから、普通にROS Kineticをインストールした状態で使えるOpenCVのクラスを使うのが筋というものです。

現行のOpenCVではMSERでキーポイント抽出できない

ほとほと困ってなんとか現行のMSERを使ったコードでビルドを通しましたが、どうやら現行のOpenCVではMSERはFeature2dクラスを継承しておらず、いわゆるキーポイント抽出をするメソッドを持ち合わせていません汗。

現行のOpenCVの仕様に合わせ、アルゴリズムをMSERからSURFに切り替え

仕方がないので現行のOpenCVの文法にそってキーポイント抽出手法をSURFに切り替えることで、本文の意図に沿ったキーポイント抽出および画像への描画ができました。キーポイント抽出についてはベースとなる考え方は共通なはずで、アルゴリズムを変えることは自体に問題はないと思います。

本文への影響

さて、今度問題なのは、ビルドを通したコードで本文を解説するよう修正が必要です。今の本文は、ビルドの通りえないコードをベースに書かれており、プログラミング解説本にして流石にこの問題を放置するわけにはいきません汗。

更に、この次のコードで特徴点マッチングをするのですが、同じく今はなきMserFeatureDetectorを使っているので、やっぱりそっちでもコードと本文を修正する必要が出てきます。

実作業の程度

ベースとなる本文はあるので、コードを入れ替えてクラスやメソッド名を修正するだけかな、とは思います。そんなに苦労はしないと思います。コードさえできてしまえばただの置換作業かなと。

ここまでやったので、残りの特徴点周りのコード作成と、本文修正案は私が出しますね。

@RyodoTanaka の範囲

日曜日中に、機械学習のところと、ROSとの連携のところ、できますか？

パット見ですが、前者はインクルードファイルの問題で解決でき、後者はすぐビルド通るんじゃないかなーと、思われます。

[TakeshiNishida]

本当にありがとうございます。

ところで、6章の diff_drive_controller.cpp の423行目　「Retreive」→「Retrieve」に修正をお願いします。

鍵カッコは、頻出するコマンドやソフトウェア名は、初出のみ鍵カッコをつけて、以降は外す、ようにしていました。また、冗長な文章は、消すと、後に続く様々なレイアウトへの影響が懸念されますので、残しています。

[MoriKen254]

test_feature_detection_matching.cpp ビルドは通るのですが、ランタイムエラーで落ちます汗。

ディスクリプタのマッチング結果を描画するところで落ちます。

ディスクリプタは空ではないこと、マッチング結果も値を持っていることまでは確認済みで、なぜかdrawMatchesで落ちます。結果得られてるんだから描画しておくれよ、という感じです。

困っています。助けてください…涙。

OpenCV Error: Assertion failed (i1 >= 0 && i1 < static_cast<int>(keypoints1.size())) in drawMatches, file /tmp/binarydeb/ros-kinetic-opencv3-3.3.1/modules/features2d/src/draw.cpp, line 211
terminate called after throwing an instance of 'cv::Exception'
  what():  /tmp/binarydeb/ros-kinetic-opencv3-3.3.1/modules/features2d/src/draw.cpp:211: error: (-215) i1 >= 0 && i1 < static_cast<int>(keypoints1.size()) in function drawMatches

Aborted (core dumped)

いろいろなサイトを見ていますが、小手先で解決できていません。

• https://docs.opencv.org/3.1.0/d5/d6f/tutorial_feature_flann_matcher.html
• http://answers.opencv.org/question/12048/drawmatches-bug/
• https://stackoverflow.com/questions/24552590/opencv-sift-surf-orb-drawmatch-function-is-not-working-well
• https://stackoverflow.com/questions/33332055/opencv-how-to-create-a-descriptorextractor-object

さすがに疲れました笑。また明日の夜にデバッグ試みます。

他環境でもチェックお願いします m( )m

西田先生 承知しました。よろしくお願いします。

[MoriKen254]

Surfのマッチングを行うコードにつき、Webのサンプルを参考に書いたものが動くことを確認しました。OpenCV2(←古いの)とOpenCV3(←Kineticを入れるとついてくる方)では、お作法が少し違うのかもしれません。

これをベースに本文のサンプル向けに書き換えればOKなので、今日中になんとかなると思います。

取り急ぎ。

[MoriKen254]

test_feature_detection_matching.cpp 実行もOK。

大変恐縮ですが、このまま機械学習に着手します。

残件1

本文ではこの後、上記cppファイルに関数を追加するような指示がなされているので、それもこのコードに加えておくべきでしょうね…。校正コメントにて、ページ数で混乱が生じている問題も解決できます。

優先度は、2番です。

残件2

機械学習とros I/Fにつき、もし未着手ならそっちを片付けてしまいます。

この状況では、優先度は1番です。 （残件1の分はROS本としてはさして重要でない処理なので、最悪本文ごと抹消できますが、こちらは消すわけには行きません。）

残件3

あと、マッチング用の画像ファイルを自力で２枚分用意させて、catkin_wsフォルダ直下に自分で画像を格納しないと使えない現状の仕様もちと問題ありです汗。ブログならともかく、お金を出している読者としては辛いものがあるかと…。

ソースコードでは、インクルードを一つ追加するのと、一行書き換えるだけでここは改善できるので、やってしまいます。本文にはほとんど影響なしです。

これは優先度3です。

補足

本件、本日締切の仕事です。後手になると出版社様との調整に影響します。可能な限り本文に影響を与えないように、ソースコード側で問題を吸収させながら、読者が気持ちよく（笑）サンプルコードを触れるレベルには持っていきましょう！

てなわけ、割り込み大歓迎です。その際はここに投稿下さい。

取り急ぎ。

[MoriKen254]

test_svm.cpp 完了。ビルドを通し、本文で意図している動作を確認。

ここで言っても仕方ないのですが、本文との違いをメモしておきます…。

• include ファイルの不備を修正
• 現行のOpenCVにはないCvSVMクラスを現行のSVMクラスに修正
• 識別用データの配列の型の不備を修正
• 関数名の名称をCamelからPascalに修正
• その他動作不良に関係する細々した修正(ベクトルの次元が合ってないとか、添字が合ってないとか、識別結果のConsole出力が計算結果と合ってないとか)。

本当に、ROS本で使うことを前提に、過去にローカルで最低一回でもビルドしてくれたのかと、問いたくなります…。

引き続き、ROS I/F に移ります。

[MoriKen254]

opencv ROS連携できました。特徴点抽出のマッチング精度向上のための補足用関数の追加を除き、ソースコードの作成・修正が終わりました。

ただ、本文のソースコードからある程度変更を加えることになってしまいました。 気になるところもあるのですが、欲は出さず、スピード重視でただ動作させることを念頭にした、最低限の修正なのですが…。

• node側のinclude ファイルの記載が「全く」ない問題を修正。
• ようやくROSが登場してきたのに、CMakeFile.txt の追加設定に関する記述が一切ない。
• ImageTransportクラスを始め、種々のROS関連のクラスについて誤った初期化手順を修正。
  • 本文のソースコードでは明らかに記述が間違っており、あのままでは「絶対に」ビルドは通りませんでした。
• ImageConverterクラスはサブスクライブもパブリッシュも両方するクラスであるにもかかわらず、「opencv_pub.cpp」とあたかもパブリッシュのみをするクラスであるような誤解を与えるファイル名である問題を修正。
  • 具体的には「opencv_ros.cpp」とした。ROSと連携していることがわかればOK。
• main側のコードは、前述したパブリッシュとサブスクライブを両方行うクラスをnodeとして実行するファイルであるにもかかわらず、「opencv_sub.cpp」と、あたかもサブスクライブの処理しか行わないようなノードであると誤解を与えるファイル名である問題を修正。
  • 具体的には「opencv_ros_node.cpp」とした。ROSと連携していること、クラスを使用してnodeとして実行していることがわかればOK。むしろ、この形態のほうがROSの慣習に準拠している。
• 本文で、上記2つのcppファイルを用意する関係で、「opencv_ros.cpp」側の宣言「opencv_ros.hpp」を用意。
  • ファイルを分ける構成を取るのであれば宣言部はヘッダ化しないと多重リンクのエラーが発生します…。
• OpenNIでKinectを接続していることを前提にしている問題を修正。
  • ローカルの画像ファイルを定周期で読み込み、円の描画位置を固定ではなく、一様分布の乱数で降った位置に描画するように変更。
  • これなら、特殊なハードウェアを接続することなく、純粋にOpenCV画像とROSメッセージ間のやり取りが定周期でリアルタイムに実現していることを確認できる。

提案

この修正につき、本文の修正も考慮すると、ページ数の増大が懸念されます。

そこで、冒頭で触れた「キーポイントマッチング精度向上の記述」を見直すのはいかがでしょう。見た目だけ修正するなら、マッチングポイントについて、スコアの高いものだけに絞れば、かなりいい結果が得られますし、その処理を入れるだけなら元々のソースコードに数行追加するだけで完結し、構成もシンプルになります。

これでページ数の増減はプラマイゼロくらいになると思います。

[MoriKen254]

本文の修正範囲　見積もり結果

ようやく全体が終わったので、修正項目の見積もりが出せます。必要に応じ、先行して出版社様とのご調整をおすすめ頂ければスムーズになると思います。

• コードの差し替え（7章全編）
• 差し替えたコードに対応する本文中のクラス名、変数名、メソッド名の置換
• 執筆段階で使用しているOpenCVのバージョンを明記。
• サンプルコードの実行に関するコマンドを追加する必要あり。
  • そもそも各サンプルを実行するためのコマンドが一切書かれていないので、読者は何をどうすれば本文に解説のあるプログラムを実行できるのかがわからない記述になっている。
  • roscore 立ち上げてね、とか。
• 画像ファイル名の修正・統一化。
  • 途中でころころファイル名が変わるので、表示される画像が入れ替わったりして、ユーザとしては困るし混乱する。
• catkin package作成の手順を追加。
  • これも一切の説明がないのはちょっと…。本文に数行入れればOK。
• CMakeLists.txtの設定に関する説明追加。
• ROS、OpenCV連携周りの説明は全体的に修正が必要。

見積もり結果への所感

大変恐縮なのですが、西田先生の予想より、ボリューム大だと思われます…。コードを修正した私自身としてもあまりにも想定外で…。出版社様にもご無理を通すご調整もあり、あまりご負担をかけるのも申し訳ないと思うのですが…。それも重々承知の上なのですが、それでもこれはなんとかしないと、今のままでは７章はすでに破綻している状況だと思います。

そもそも、最終校正の段階でソースコードの品質に甚大な欠陥があったのが本質的な問題です。それを著者・出版社双方がここまで管理できていなかったという事実は否定できないので、ここは痛み分けをしてでも修正できないかと存じます。（出版社側に技術的なレビューを求めるのが酷なのは承知で、最低限必要な管理体制を構築するよう著者側に指示ができていなかったor監督に不備があったという点では、出版社側の管理責任もあるはず。）

学生や技術者にお金を払って購入いただいたのに、ビルドが通らない、通ってもまともに動かないコードを世に出しまう状況になるよりは、少し泥を被ってでも膿は出すほうがベターと思います。むしろ、この段階で見つけることができて幸運だったと言っても過言ではない状況だと思います。

最終校正より前の段階で全編を確認できていなかった私にも、2ndオーサとしての確認不足の責任がございます。 その責任を取る上でも、修正案は早急に当方が作成します。できるだけ修正は少なくなるよう、本文の構成面での工夫も考えますので、ご検討頂ければ幸いです。

本文修正案の納期

すみません、本文の明日以降になります。当初土日で完了するという見積りは、ソースコードのビルドが通ることを前提にしていましたので。

お時間がないのは把握しておりますので、できるだけ早くやりますが、夜間まで着手できないので、火曜日まで時間をいただけませんか？

大変恐れ多いのですが、本文に記載された（しかも納品されていないのでゼロベースで全て手打ちする必要がある）全てのソースコードについて、文法、3rd PartyのVer.管理、APIの利用方法、データのハンドリング方法、CMakeFileの設定不良等など、ソフトウェアの基本からアプリケーションの適用に至るまで、様々な不備が混在しており、全て解決するのに大分苦労をしていまいました…。

ここまで確認をしていなかった私にも責任がございます。今後の教訓にしたいと思います。

よろしくお願いします。

[TakeshiNishida]

遅くまで本当にありがとうございます。ぜひ修正しましょう。必要な情報があればお知らせください。

[MoriKen254]

コードを「動く状態」から整形＋ROS連携させました。

これで、最初のOpenCVの基本以外の全てのサンプルプログラムが、ROSノードとして実行されるようになりました。画像トピックをパブリッシュしてimage_viewで見る、RVixで画像トピックを閲覧する、というサンプルも置いてはあります（ROSでコンピュータビジョンをやる章で、ここに一切触れていなかったことのほうが、？？？？？だったので…。PCLの章でポイントクラウドをRVixで可視化しない、というくらい違和感のある話です…。）。

サンプルは置いてはいますが、大した話ではありません。roslaunchで起動するよう誘導すれば勝手に連携するので、本文にもさして影響はありません。

[MoriKen254]

本文の訂正は明日します。

また、会社側で私の著者紹介欄に会社名を入れられるかについて、技術部門と知財に話しているところです。進捗は、節目で報告します。

よろしくお願いします。

[MoriKen254]

先日火曜日までに一旦当方の作業を完了させると宣言しましたが、日をまたいでしまいました。失礼いたしました。

6章

＞ 6章の diff_drive_controller.cpp の423行目　「Retreive」→「Retrieve」に修正をお願いします。 当該コードはROS本家のパッケージなので、本家の人のタイポと思われます。こちらのパッケージを修正する必要はなさそうです。

7章

修正につきご理解頂き、本当にありがとうございます。こんな最終校正の段階で、ようやっと落ち着くとお思いになっていたであろう先方や先生にも負担が発生するので、申し訳ないです。

さて、一旦全体校正しました。先述した理由によりソースコードは全て差し替えました。Ubuntu 16.04 ROS Kinetic でcatkin_makeが通り、全て正常動作することを確認済みです。

本文の修正案はDropBoxに格納します。ご検討下さい。レイアウト修正は先方の負荷になります。変更点過多であれば、ご指摘頂き次第対応も致します。

• Dropbox/rosbookworkspace/最終校正/180711_7章校正 に必要ファイル格納
  • pdf内で、赤字が追加、取り消し線が削除です。
  • 取り消し線や赤字を埋め込むためのproofread用のstyファイルも入れてあります。
  • もし格納されたtexをコンパイルするのであれば必要なので。念の為。

　　

• 特徴点抽出

  • その後、MSERを発見。当初の本文に意向に沿うようコードを編集。正常動作を確認。
  • SURFやSIFTはデフォルトではライセンスの問題で使えないと注釈があるが（一般的にはそれは正しいが）、ROS KineticにバンドルされたOpenCVを入れればそれらもデフォルトで使えますので、その旨は訂正。

• コード

  • すみません、元々ビルドの通らないコードで直さざるを得ないのであればと、ついでにROS本らしい作りにしたく、処理結果の画像はROSトピックで飛ばし、image_viewやRVizでサブスクライブさせることにしてしまいました。
  • それを行うためのlaunchファイルやrvizファイルも追加しました。
  • 本文にCMakeLists.txtへの追記事項を明記しました。

• その他

  • ページ数を増やさないように、新規画像はありません（ROSとの連携サンプル、エッジ検出については）。
  • 現段階でKineticを入れるとOpenCV 3.3.1が入ることを、それを前提としたサンプルであることを明記。
  • ページ数が増大しすぎないように、優先度の低い記述はfootnoteに逃したり（畳み込み演算の説明であれもこれもと当該節とは無関係のクラスについても語り初めたところとか）、サンプルが一切ない節については削除したりしました（透視投影モデルとホモグラフィー変換辺り）。
  • 社内調整は最終校正のゲラを用います。私が執筆担当した部分を抜粋して社内の人間が確認することになります。オープンソースが題材なので問題は無いと思います。急いでもらうようお願いしていますが、7月中くらいは時間が必要かもしれません。

バタバタさせてしまい申し訳ございませんが、よろしくお願い致します。

[TakeshiNishida]

ありがとうございます。本件を今日から最終校正原稿に反映させます。 ところで、修正の終わった？追加した？7章のソースコードをUPしてもらっていいでしょうか？

[TakeshiNishida]

新しく追加された「7.3 画像のパブリッシュとサブスクライブ」 を、「7.2.2 画像のパブリッシュとサブスクライブ」として、元の「7.2.2 画像への描画とイベント検出」の前に入れ込んでもいいですか？

[MoriKen254]

すみません、わがままを聞き入れて頂き感謝いたします。

質疑

ありがとうございます。本件を今日から最終校正原稿に反映させます。 ところで、修正の終わった？追加した？7章のソースコードをUPしてもらっていいでしょうか？

現在「add_chapter7」ブランチに上がっています。このコメントの直後「master」ブランチにマージします。

新しく追加された「7.3 画像のパブリッシュとサブスクライブ」 を、「7.2.2 画像のパブリッシュとサブスクライブ」として、元の「7.2.2 画像への描画とイベント検出」の前に入れ込んでもいいですか？

はい、大丈夫です。

フェールセーフの観点で

今回、ボトムアップ側の横槍でQを向上させる代償に、CとDを犠牲にするリスクが出たと認識しています。もし納期の問題で復旧等必要であればご連絡ください。準備はしてあります。

以上、よろしくお願いいたします。