SOLID未分類 SOLID for Raspberry Pi 4 (連載13)
(2023/1/30)
前回書いたサンプルプログラムや記事を、有識者の方にチェックいただきました。
今回は、そのフィードバックの内容について書きます。
前回記事の最後で「次回はSPI受信部を作っていきます。」と書きましたが、それは次回へ持ち越しという事で。。。
前回記事の最後に、「動きました！」と嬉々として波形を貼り付けましたが、SPI受信端子(SDD)がロジックアナライザから外れていました。
前回記事では、送信側処理だったため、受信端子は気にしていませんでした。
正しくはこちらの波形になります。
SDD（受信端子）にもデータが載っていることがわかります。
では、ここからは有識者レビューの結果について書いていきます。
GPIO8,9,10,11の端子機能をALT0に設定する処理において、すべてALT0に設定するのであればループが使えるよ、との事で、格好いいコードになりました。
元々のソースコード：
pub fn init() {
const GPIO_NUM8: usize = 8;
const GPIO_NUM9: usize = 9;
const GPIO_NUM10: usize = 10;
const GPIO_NUM11: usize = 11;
//GPIO8,9,10,11の端子機能をALT0に設定
gpio_regs().gpfsel[GPIO_NUM8 / gpio::GPFSEL::PINS_PER_REGISTER].modify(
gpio::GPFSEL::pin(GPIO_NUM8 % gpio::GPFSEL::PINS_PER_REGISTER).val(gpio::GPFSEL::ALT0),
);
gpio_regs().gpfsel[GPIO_NUM9 / gpio::GPFSEL::PINS_PER_REGISTER].modify(
gpio::GPFSEL::pin(GPIO_NUM9 % gpio::GPFSEL::PINS_PER_REGISTER).val(gpio::GPFSEL::ALT0),
);
gpio_regs().gpfsel[GPIO_NUM10 / gpio::GPFSEL::PINS_PER_REGISTER].modify(
gpio::GPFSEL::pin(GPIO_NUM10 % gpio::GPFSEL::PINS_PER_REGISTER).val(gpio::GPFSEL::ALT0),
);
gpio_regs().gpfsel[GPIO_NUM11 / gpio::GPFSEL::PINS_PER_REGISTER].modify(
gpio::GPFSEL::pin(GPIO_NUM11 % gpio::GPFSEL::PINS_PER_REGISTER).val(gpio::GPFSEL::ALT0),
);
}
ループを使ったソースコード：
pub fn init() {
for pin in 8..=11 {
gpio_regs().gpfsel[pin / gpio::GPFSEL::PINS_PER_REGISTER].modify(
gpio::GPFSEL::pin(pin % gpio::GPFSEL::PINS_PER_REGISTER).val(gpio::GPFSEL::ALT0),
);
}
}
さらに、全部がALT0でない場合、個別に変更できるようにするには以下。
pub fn init() {
for (pin, gpf) in [
(8, gpio::GPFSEL::ALT0),
(9, gpio::GPFSEL::ALT0),
(10, gpio::GPFSEL::ALT0),
(11, gpio::GPFSEL::ALT0),
] {
gpio_regs().gpfsel[pin / gpio::GPFSEL::PINS_PER_REGISTER]
.modify(gpio::GPFSEL::pin(pin % gpio::GPFSEL::PINS_PER_REGISTER).val(gpf));
}
どのように書くか、は、書き手の趣味の範囲もあるかと思いますが、筆者が 思考停止状態で 動くことを最優先とし、べとーっと書いたコードが格好良くなりました。
さらに、別関数に分けるアプローチもありますが、キリがないので残念ながら割愛します。
前回、「こういう事かと自分なりに理解しました。」と書いた部分について、解説を頂きました。
[該当箇所]
----------------
・spi_regs().clk.write( の部分：
自分解釈：spi_regs(register_structs構造体)のclkメンバに書き込みをせよ。
・spi::CLK::CDIV.val(CLKSET) の部分：
自分解釈：書き込む対象は spi::CLKクラスのCDIVメンバで、その値は .val()で操作すること
----------------
[解説]
----------------
ここではspi::Registers構造体のclkフィールドに対して writeメソッドを呼び出しています。
(spi::Registers構造体はregister_structsマクロを使用して定義されています。)
spi::CLK::CDIVはCLKレジスタのCDIVビットフィールドのビット範囲を表す定数です。
このビット範囲はField構造体で表されています。
Field構造体のvalメソッドを呼び出すと、そのビット範囲に代入したい値を関連付けたFieldValue構造体が得られます。
最後にwriteメソッドを呼び出すことにより、FieldValue構造体で表されるビットフィールドの新しい値が、clkフィールドで表されるメモリ番地に書き込まれます。
----------------
ふむむ、、、
まず自分が、クラスだとかメンバだとか、なんとなく適当に使っていることに気が付きました。
C/C++言語を本当の意味で正しく使えている人にとっても、違和感のある文章だったなと反省しております。
以下のコードについて、ちょっと気になるとのご指摘。
//CS : 00 = Chip select 0 -> CS.bit0 and 1
//CPOL: Clock Polarity 1 = Rest state of clock = High -> CS.bit3
//CPHA: Clock Phase 1 = First SCLK transition at beginning of data bit. -> CS.bit2
spi_regs().cs.modify(
spi::CS::CS::ChipSelect0,
);
spi_regs().cs.modify(
spi::CS::CPOL::RestStateIsHigh,
);
spi_regs().cs.modify(
spi::CS::CPHA::FirstSclkTransitionAtBeginningOFDataBit,
);
これについての修正案を、以下2点頂きました。
①rustfmtを使う
rustfmtというツールがあり、コミュニティのコードスタイルに合わせて自動的にフォーマットしてくれます。
https://doc.rust-jp.rs/book-ja/appendix-04-useful-development-tools.html
SOLID-IDEではCtrl-K -> Ctrl-Dで呼び出せます。
さっそく試してみます。
単に、ソースコードファイル上のどこかをポイントし、Ctrl-K を押したのちCtrl-Dを押すだけです。
わっ。
ソースコードが自動で全部整形された！
で、その該当部分は、以下のようになりました。
//CS : 00 = Chip select 0 -> CS.bit0 and 1
//CPOL: Clock Polarity 1 = Rest state of clock = High -> CS.bit3
//CPHA: Clock Phase 1 = First SCLK transition at beginning of data bit. -> CS.bit2
spi_regs().cs.modify(spi::CS::CS::ChipSelect0);
spi_regs().cs.modify(spi::CS::CPOL::RestStateIsHigh);
spi_regs()
.cs
.modify(spi::CS::CPHA::FirstSclkTransitionAtBeginningOFDataBit);
}
（ちょっとびっくりした。。。）
②+演算子で結合
FieldValueは+演算子で結合できるとの事。
spi_regs().cs.modify(
spi::CS::CS::ChipSelect0
+ spi::CS::CPOL::RestStateIsHigh
+ spi::CS::CPHA::FirstSclkTransitionAtBeginningOFDataBit,
);
式が一つになりますね。
今回は、せっかくなので、自動成形された①を採用したいと思います。
こちらも、動かすことを最優先とした結果、筆者のコードは以下でした。
let mut retval = false;
while retval == false
{
retval = spi_regs().cs.is_set(
spi::CS::TXD
);
}
これは、以下のように一文で書けます。
while !spi_regs().cs.is_set(spi::CS::TXD) {}
ここについて、先ほどのCtrl-K -> Ctrl-Dでは以下のように整形されています。
let mut retval = false;
while retval == false {
retval = spi_regs().cs.is_set(spi::CS::DONE);
}
ですが、ここについては、教えて頂いた書き方にしたいと思います。
補足：
C言語でも、書こうと思えば一行でいろいろな意味を入れ込むことができますよね。
個人的には、コードをまとめて書く事が良いのか悪いのかという事では測れないような気がします。
どちらの書き方が可読性が良いのか、結局は人それぞれであり、その辺はRustでも無理に強制するような事はしていないのですね。
ビットのセット＆クリア、も、既に備わっています。
レジスタ操作のために使用しているtock_registers::register_bitfields! マクロ (厳密にはその内部マクロ) が、定数 SET, CLEAR も定義しています。
以下のレジスタへのアクセスは、SETやCLEARを使って、余計なデータの定義はやめてしまいましょう。
・spi::CS::TA.val(CS_TA_ACTIVE), ⇒ SET が使える
・spi::CS::CLEAR_TX.val(0x01), ⇒ SET が使える
・spi::CS::TA.val(CS_TA_INACTIVE), ⇒ CLEARが使える
SET,CLEARを使うように変更しました。
spi_regs().cs.modify(spi::CS::TA::SET);
:
spi_regs().cs.modify(spi::CS::CLEAR_TX::SET);
:
spi_regs().cs.modify(spi::CS::TA::CLEAR);
Rust関連ではありませんが、送信部のフローについて、指摘ありました。
（コードレビューって大切ですね、と実感した次第です）
------送信フロー---------
①CSレジスタのTAビットを1にする。
②FIFOに送信データを書く（送信データ：ライト対象の加速度センサのレジスタ値”0x2c”）
③CSレジスタのTXDビットが1になるまで待つ
（待つ処理の前に5usほどウエイトを入れる。そもそも0になる前にチェックをしないようにするため。ウエイト値は後で見直す。）
④CSレジスタのCLEARビットに0x01を書いてFIFOクリア
⑤FIFOに送信データを書く（送信データ：先ほどのレジスタへのライト値”0x0b”）
⑥CSレジスタのTXDビットが1になるまで待つ
（待つ処理の前に5usほどウエイトを入れる。そもそも0になる前にチェックをしないようにするため。ウエイト値は後で見直す。
⑦CSレジスタのCLEARビットに0x01を書いてFIFOクリア
⑧CSレジスタのDONEビットが1になるまで待つ
⑨CSレジスタのTAビットを0にして送信完了。
------------------------
[レビュー指摘]
TXDは送信FIFOに1バイト以上の空きがあるかを示しています。送信完了を検出できるわけではありません。
はっ、しまった。これは間違いでした。
③でFIFOに空きがあるかどうかをチェックしているだけで、データがすべて出ていった事を確認しているわけではありません。
先のデータがFIFO内にまだ残っている可能性がありますので、④でCLEARしてしまったらいけません。
CLEARせずに、次のデータを書き込まないと。
同じ理由で、⑦と⑧は順番を入れ替えるべきです。
さらに、⑥についても不要です。
そのあとにFIFO書き込みがあるわけではないので、FIFOの状態をチェックする必要はありません。
正しくは以下。
------送信フロー---------
①CSレジスタのTAビットを1にする。
②FIFOに送信データを書く（送信データ：ライト対象の加速度センサのレジスタ値”0x2c”）
③CSレジスタのTXDビットが1になるまで待つ
④FIFOに送信データを書く（送信データ：先ほどのレジスタへのライト値”0x0b”）
⑤CSレジスタのDONEビットが1になるまで待つ
⑥CSレジスタのCLEARビットに0x01を書いてFIFOクリア（念のための処理）
⑦CSレジスタのTAビットを0にして送信完了。
------------------------
このように動かしてみたところ、Waitの処理は必要ありませんでした。。。申し訳ない。。。
動作させ、波形を取得しました。
FIFOの間違った処理を正したため、前回の波形よりも短い時間で送信完了しています。
いろいろと助言頂き修正したソースコードは以下です。
#[no_mangle]
pub extern "C" fn slo_main() {
println!("Starting SPI control.");
spi_gpio_control::init();
spi_control::init();
spi_control::senddata();
}
mod spi_gpio_control {
use bcm2711_pac::gpio;
use tock_registers::interfaces::{ReadWriteable, Writeable};
fn gpio_regs() -> &'static gpio::Registers {
// Safety: SOLID for RaPi4B provides an identity mapping in this area, and we don't alter
// the mapping
unsafe { &*(gpio::BASE.to_arm_pa().unwrap() as usize as *const gpio::Registers) }
}
pub fn init() {
for pin in 8..=11 {
gpio_regs().gpfsel[pin / gpio::GPFSEL::PINS_PER_REGISTER].modify(
gpio::GPFSEL::pin(pin % gpio::GPFSEL::PINS_PER_REGISTER).val(gpio::GPFSEL::ALT0),
);
}
}
}
mod spi_control {
use bcm2711_pac::spi;
use tock_registers::interfaces::{ReadWriteable, Readable, Writeable};
fn spi_regs() -> &'static spi::Registers {
// Safety: SOLID for RaPi4B provides an identity mapping in this area, and we don't alter
// the mapping
unsafe { &*(spi::BASE_SPI0.to_arm_pa().unwrap() as usize as *const spi::Registers) }
}
pub fn init() {
const CLKSET: u32 = 0x00cb;
//SPIのCLKレジスタにSPI周波数を設定
spi_regs().clk.write(spi::CLK::CDIV.val(CLKSET));
//CS : 00 = Chip select 0 -> CS.bit0 and 1
//CPOL: Clock Polarity 1 = Rest state of clock = High -> CS.bit3
//CPHA: Clock Phase 1 = First SCLK transition at beginning of data bit. -> CS.bit2
spi_regs().cs.modify(spi::CS::CS::ChipSelect0);
spi_regs().cs.modify(spi::CS::CPOL::RestStateIsHigh);
spi_regs()
.cs
.modify(spi::CS::CPHA::FirstSclkTransitionAtBeginningOFDataBit);
}
pub fn senddata() {
//(1)CSレジスタのTAビットを1にする。
spi_regs().cs.modify(spi::CS::TA::SET);
//(2)FIFOに送信データを書く（送信データ：ライト対象の加速度センサのレジスタ値;
とりあえず0x2cと固定しておく。）
spi_regs().fifo.write(spi::FIFO::DATA.val(0x2C));
//(3)CSレジスタのTXDビットが1になるまで待つ
while !spi_regs().cs.is_set(spi::CS::TXD) {}
//(4)FIFOに送信データを書く（送信データ：先ほどのレジスタへのライト値）
spi_regs().fifo.write(spi::FIFO::DATA.val(0x0b));
//(5)CSレジスタのDONEビットが1になるまで待つ
while !spi_regs().cs.is_set(spi::CS::DONE) {}
//(6)CSレジスタのCLEARビットに0x01を書いてFIFOクリア
spi_regs().cs.modify(spi::CS::CLEAR_TX::SET);
//(7)CSレジスタのTAビットを0にして送信完了。
spi_regs().cs.modify(spi::CS::TA::CLEAR);
}
}
格好いいコードになりました！
（それにしても、Ctrl-K -> Ctrl-Dの自動フォーマット機能にはビックリした。。。）
次回は受信処理を書いていこうと思います。