Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rtmouse.cの分割(リファクタリング) #92

Merged
merged 26 commits into from
Nov 15, 2024

Conversation

YusukeKato
Copy link
Contributor

@YusukeKato YusukeKato commented Nov 5, 2024

What does this implement/fix?

すべての処理がrtmouse.cに記述されているため、メンテナンスしやすいようにファイルを分割します。

主な変更内容は下記の通りです。

  • rtmouse.crtmouse_main.cへ名称変更
  • rtmouse_main.cには変数の定義と主要な関数の定義を記述
  • rtmouse_dev.cにデバイス関連の関数を記述
  • rtmouse_spi.cにSPI関連の関数を記述
  • rtmouse_i2c.cにI2C関連の関数を記述
  • rtmouse_gpio.cにGPIO関連の関数を記述
  • rtmouse.hで外部変数をexternで宣言
  • ファイル分割に対応してMakefileを修正
  • .test/lint.shでフォーマットをチェックするファイルを追加・修正

ファイル構成は下記の通りです。

src/
└── drivers
    ├── Makefile -> Makefile.header_from_apt
    ├── Makefile.header_from_apt
    ├── Makefile.header_from_source
    ├── rtmouse.h
    ├── rtmouse_dev.c
    ├── rtmouse_gpio.c
    ├── rtmouse_i2c.c
    ├── rtmouse_main.c
    └── rtmouse_spi.c

Does this close any currently open issues?

しません。

How has this been tested?

下記の環境ですべてのサンプルプログラムが動作することを確認しました。

  • Rapberry Pi 4B
    • Ubuntu 24.04 LTS
    • Ubuntu 22.04 LTS
    • Raspberry Pi OS 64bit
    • Raspberry Pi OS 32bit

CIのビルドテストのチェックが通ることを確認しました。

Any other comments?

Checklists

@YusukeKato YusukeKato self-assigned this Nov 5, 2024
@YusukeKato YusukeKato added the Type: Refactoring A code change that neither fixes a bug nor adds a feature label Nov 5, 2024
@YusukeKato YusukeKato marked this pull request as ready for review November 7, 2024 09:42
@YusukeKato YusukeKato requested a review from KuraZuzu November 8, 2024 01:17
@KuraZuzu
Copy link
Contributor

Raspberry Pi 4B(4GB)上のUbuntu Server 24.04(6.8.0-1013-raspi)でビルドとサンプルプログラムの動作を確認しました。

Copy link
Contributor

@KuraZuzu KuraZuzu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ソースコード中に4箇所(内容は同じ)のコメントをしました。
既に実装された関数に関するもので、今回のPRの本質とは少しばかりずれていますが、不具合を引き起こす可能性があると思ったためコメントしています。今回のPRで修正するべきでないと思った場合は無視してももらってもかまいません。その場合は、こちらで別途PRを作成します。

src/drivers/rtmouse_dev.c Show resolved Hide resolved
src/drivers/rtmouse_dev.c Show resolved Hide resolved
src/drivers/rtmouse_dev.c Show resolved Hide resolved
src/drivers/rtmouse_dev.c Show resolved Hide resolved
@YusukeKato
Copy link
Contributor Author

動作確認ありがとうございます。
また、mutex_unlock(&dev_info->lock);追加の提案ありがとうございます。
たしかに修正必要そうですが、少し煩雑になってしまうので別PRに分けるようにお願いいたします。

@KuraZuzu
Copy link
Contributor

承知しました。
実装は問題なく、動作確認もできたのでLGTMです。

@KuraZuzu KuraZuzu merged commit 5bdf8e3 into master Nov 15, 2024
11 checks passed
@KuraZuzu KuraZuzu deleted the refactoring/divide_cfile_20241105 branch November 15, 2024 08:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Refactoring A code change that neither fixes a bug nor adds a feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants