*******************************************************************
SWEST8
S1-4-4「Surveyor Project Hamana-3最終調整」議事録
会場：50人程度の広さ、講演形態(参加者は10名程度)
(今回はWitzの新入社員研修ということで、例年とは違い和室で
の調整作業ではなかった)
内容：Witzソースコードレビュー
仕様書とソースコードを来場者に配布・スクリーン表示して説明。
前に説明者と記録者の2名が座り、説明者が終始説明を行った。
参加者は、内容に関して意見があればその都度指摘する、といった流れ。
*******************************************************************
===================================================================
仕様書の説明からスタート
以下、
A,B：来場者
C,D：大西, 岸田
===================================================================
説明者：まずは仕様書から見ていきたいと思います。
—「ジャイロセンサ デバイスドライバ仕様書ver0.72」をスクリーンに表
示。
○ジャイロの状態遷移図、状態遷移表
—早速厳しいご指摘がとびはじめる。
A：初期化はどうなるの？
初期値不定だと、スタートから始まるってこともありうる？・・・
A：(状態遷移表で)初期値が不定で、全部INITにいくんだよね。状態が不定
であれば、すべてからINITにいけるんだからそのようにしなくては？
（INVALID->INITが×になっていた）
説明者：間違いでした(ここは後々コードで修正されていることが判明する)。
—このあたりでフォローが入る。
B：初期状態はSLEEP、INIT、STARTのどれかだといえば矛盾はないよね。
そして全状態というのをこの３つにすれば・・・
説明者：ありがとうございます。
—さらに説明は続く。
○データ型、およびデータ構造
○エラーコード
○ジャイロセンサインタフェース詳細
—このあたりで再び指摘が入る。
A：NON_STOPとデバイスの制御失敗はどんな関係があるの？
説明者：エラーコードに戻ってみます。エラーコードに書いてないものを
書いてしまい混乱させてしまいました。停止できないときNON_STOP
を返すと言うのは間違いで、エラーを返すと解釈してください。
—さらに説明が続き、仕様書の説明はここで終了。
===================================================================
コードレビューに入る前に注意事項が説明される
===================================================================
C：コードレビューのお約束を書いておきたいと思います。
まず、新入社員がつくったのでかなり下手です。
経験豊かな皆さんからご意見を伺いたいと思います。
仕様がどうのこうのというのではなく、一見しておかしいところを指摘
して細かいところは彼らに聞いて欲しいと思います。
次に、上手なコードというのは何ですかというのを教えていただきたい
と思います。彼らが作ったものがどうしたら良いコードになるかという
のを指摘していただきたいと思っています。(きれい、読みやすいとか)
いじめてあげてください、というとおこられるんですけども、弊社の中
でもレビューをしたんですが、すべてを反映できていません。彼らも忙
しいので、できていません。また、2時間と短いのでこのあと、303でコ
ードに関して個別に意見を頂きたいと思います。
最後に、厳しい目で見ていただきたいと思います。
ヘッダのところを簡単に説明した後、コードのところ何かご意見いただ
きたいと思います。
===================================================================
そしてコードレビューに
===================================================================
説明者：ではコードに入りたいと思います。
今回はソースコードをハードウェア層とコントロール層の2層にわ
けました。コントロール層はハードを触らない部分、レジスタな
どハードウェアに関わる部分がハードウェア層です。
—説明をはじめようとしたところで指摘が入る。
D：ソースファイルを見ているのかとか、資料のどこかを説明していただけ
ますか？
説明者：まず、1枚目の紙になりますが、ヘッダファイルを右と左に分けて
載せてあります。左がコントロール層、右がハードウェア層にな
ります。1枚目の左から右と2枚目の表の左半分の真ん中から上ま
でコントロール層のCファイル、そこから2枚目の裏にかけてハード
ウェア層のCファイルです。
—そして説明に入る。
【コントロール層のヘッダファイル】
○インクルードファイル
○定数
D：どんどん質問してください。いろいろとあると思うんですが。
えーでは私から、0から状態定義を並べてるのはどうしてですか？
説明者：えー・・・
—少しつまったところで優しくフォローが入る。
D：ポリシーを説明していただければいいんですよ。答えはないので。
あなたの設計思想はどうなってるか、設計と矛盾がありませんか？
気にせずに考えを言ってくれればいいですよ。敵とか味方とかないです
からね。
*** コーディネータD コメント
「答えはないので」は「答えではないので」
ここで求めているのは「答えではない」と言いたかったんですが，
そう伝わらなかったようです．
# それぞれに選択すべき最適な解らしきものはあると私は考えています
*** ここまで
説明者：エラーコードは正常終了というのを第一にあげるべきだと考えま
して、エラーはあとからでも追加できますので、まずは終了を並
べるという意味で0からになっています。
D：INVALIDがなぜ先頭なんですか？
A：他と性質が違うからって言っちゃえばいいんですよ。
説明者：他のものと性質が異なるものと考えていますので、先頭に・・・
D：先頭が一番良く分からない。計測可能状態(START)と初期化状態(INIT)
の2つが望ましい状態ですよね。なぜこれらが先頭じゃないのかな。ど
うつくりたいか、というのがあって、区別をしたいからなんですよね。
—とりあえずここまでにとどめて説明が続く。
○ジャイロセンサ計測値構造体
A：えっと、gyr_returnに入るものは何ですか？
説明者：エラーコードと実行結果は対になっておりまして、エラーコード
のどれかが必ず入ります。
—このあたりで消し忘れていたコードが発見され、それに対して謝罪が
あった。
○関数
A：なんでread関数はポインタで値をよむの？戻り値では？
D：答えがないのならば素直に言えばいいですよ。
説明者：そこまで深く考えずに設計を組んでしまいました。
【ハード層のヘッダファイル】
○インクルードファイル
○定数
—NULLポインタの説明をはじめる。
A：このNULLっていのはdef.hにはいってないの？
説明者：なかったのでつくりました。
—比較演算用の説明。
—DIN入力チャンネルの説明。
このあたりから質問がとびかう。
A：ダミーデータをFFしたのは？
説明者：オシロでみにくかったので一定値のFFを設定しました。
D：本来とらない値にするのが普通ですが・・・FFとるのにFFにしちゃった？
説明者：はい・・・。
—SCI測定値の説明。
D：SCI2っていうのは何に設定する値？
説明者：Rxが送信、Txが受信を表しています。
D：SCI2っていうレジスタを定義してあるんですよね？
そのレジスタに対する設定値ですよね？
説明者：はい。
A：設定に使うための変数なんだけど、Rxだけ反転してつかいたいんだから
ここでコメントにかいておかないと。どのビットが何をするためのビット
かコメントをかいておけばいいとおもいますよ。
E：Rx-bitとか名前つかってるけど、ビットの値なのかビットの位置を表し
ているのかぱっと見でわからない。統一されてればいいですが。
説明者：ビットの値で統一しています。
C：bitがついてるのとついてないのは差があるの？
ついてないのは設定値（そのまま書き込む）で後で意味があるので設定
したいのは定義値？ごっちゃになってるんじゃないの？
説明者：はい、ごちゃごちゃになってしまっています。
後で反転しないとうまくいかないという形になってしまいました
。ソースコードとしてはわかりにくくなってしまったと思ってい
ます。
A：じゃぁどうするのまでいわないと。分けるの？
説明者：指定のところと全体の設定値などとごちゃごちゃなので分離しな
いととは思っています。
A：思ってますじゃなくてどうするのかいいましょう。
説明者：分けます。
D：TE(transmit enable)、RE(receive enable)とあるんだからTEREいらない
のでは？
説明者：はい、削除します。
D：じゃなんで用意したの？
説明者：一緒に設定できるからです。しかし、後で読んでから結局なんだっ
たか分からなくなって誤解を招くものになってしまいました。
—さらに説明が続く。
D：割り込みますが・・・オーバランってなんですか？(SCR2_ORER_BITについ
てきちんと理解しているかの確認)
説明者：宿題にさせていただきます。
D：後で303(部屋)でこうなんですよって発表するんですよね？
説明者：そうさせていただきます。
○関数
—特に質問はなかった。
【コントロール層のCファイル】
○インクルードファイル
A：最初のインクルードファイルのstrh・・・というのは、ハードウェアに
固有の設定ですよね？ではドライバでインクルードするっていう方法も
あったと思うんですが・・・
説明者：宿題にさせてください。
○グローバル変数
B：すいません。グローバル変数だって書いてあるんですけど名前が分かり
にくいですよね。
説明者：こうこうこうで・・・・・・
A：グローバルだというのがわかるようにつけるかどうかを言わないと。
説明者：採用させていただきます。
D：宿題をもちかえって直したものをテストして明日とばすんですよ。
A：そんなリスキーなことするんですか？
D：どうするのかここにいるみなさんにいわないといけないんですよ。
先ほどのご指摘を直させていただきたいんですが、現在のところ動作に
影響するというものは認識しておりませんので、明日の計測には直さな
いものをしようとか、レベルを自分でいってしまえばいいんですよ。
—ここでそのレベルについて議論がしばらくある。
説明者：3段階用意したいとおもうんですが・・・
レベルA—ただちに変更を要求するもの
レベルB—変更すべきだが急を要さないもの
レベルC—あくまで表記が分かりにくいので動作に関係ないもの
D：急ってなんなのかわかんないなぁ、アドバイスを頂いたら？
説明者：レベルBがわかりにくいんですがアドバイスはありますか？
C：絶対にしなくてはならないかそうでないかに分ければいい。あなたの定
義をすればいいんですよ。社内の定義があればそれを言わないといけな
い。社内でポイントが決まっているはず。この場なら、
レベルA—絶対直さないといけない
レベルB—直した方が良いけど、今回はやらない
レベルC—直さなくても良い
とかでいいと思います。
—その後もいろいろあり結局・・・
レベルA—現段階では中止せざるを得ない→すぐに直さないと飛ばせない
レベルB—直さなければならないが、打ち上げには問題ない
レベルC—問題はないが、変更はした方が良い
レベルD—質問を受けました場合に、解決した（理解を得られた）
に落ち着き、今回はレベルBに当たるということだった(つまりこの日は
直さないということに)。
○関数
—初期値が設定されていない変数が・・・
A：なぜ値を入れないの？
説明者：現段階では判断しかねるので・・・
A：でもわからないものを定義してますよね。考え方なんですが、どういう
考えなんですか？今の定義だと未定義エラーでいんじゃないの？
C：たとえばですね、最初のOKをいれてエラーがきたらそれをかえすとか、
意図があってやるもんですよね。私は最初にエラーをいれて、初期化忘
れてもエラーになるので・・
A：何かわからないってものを定義してるんだからそれを入れればいんじゃ
ないですか?
説明者：はい。
—すべての状態から終了を実行可能(仕様とちがう・・・)
C：どっちが正しい？
説明者：ソースコードが正しくて・・・
C：仕様書どおりに作れていないってことになるよね・・・仕様書のとおり
にかいたら矛盾がありましたでいいじゃない。
説明者：はい。直しておきます。
===================================================================
このあたりでタイムアップ。→ 303へ移動することに
===================================================================
C：最後に反省点はありますか？
説明者：説明が不足・・・
C：ジャイロ、ジャイロセンサ、階層構造、資料がどういう内容かを最初に
説明していないので何を聞いても理解しにくい。どういう内容でどんな
ことを説明しているのか？ジャイロとジャイロセンサが何を意味してい
るのか分からないのではじめての人に対しては分けなくてはならないん
ですよ。
A：でもすごいわかりやすかったですよ。問題点がすぐでてきたので。
（温かいフォロー）
C：火曜日にコードレビューすることを決定しましたので・・・こちらにも
不備がありました。私も勉強になりました。
説明者：ありがとうございました。
===================================================================
和室313(になったようだ)で続き
15名程度(壁にスライドを表示)(部屋半分は宴会状態)
以下、E,Fは来場者
===================================================================
○ジャイロセンサの初期化関数
E：どの程度のレビューをしましょうか？
細かいところだとインデントとかも違うし・・・でも時間たりないよね
？致命的なところいってほしいとかいわないと終わんないかと思ってま
すよ。終わらないですよね。
説明者：では細かいところは大目に見ていただきたいと思います。
【ハードウェア層のCファイル】
○ジャイロセンサ初期化関数
F：1ビット期間待ちはどうやって実現してるの？
説明者：アセンブラでNOP（空処理）を繰り返して・・・
E：コンパイラの方が最適化をしてそういったものをとばしてしまわないか
といのうをどうやって確認したかというのを聞きたいと思うんですよ。
まあ今は時間がないのでその辺はいいと思います。
—さらに議論は続いていく・・・この時点で12時頃。
議事録はここまで。」