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.hを追加(リファクタリング) #91

Merged
merged 16 commits into from
Oct 29, 2024

Conversation

YusukeKato
Copy link
Contributor

@YusukeKato YusukeKato commented Oct 28, 2024

What does this implement/fix?

リファクタリングの一環として、rtmouse.cで定義していた定数などを新しく作成したrtmouse.hへ移します。
また、rtmouse.hとは関係ありませんが、使用していないmotor_motion_*関数も削除しています。

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?

複数のCファイルに分割するところまでは本PRでは行いません。

Checklists

@YusukeKato YusukeKato added the Type: Refactoring A code change that neither fixes a bug nor adds a feature label Oct 28, 2024
@YusukeKato YusukeKato self-assigned this Oct 28, 2024
@YusukeKato YusukeKato marked this pull request as ready for review October 29, 2024 02:17
@YusukeKato YusukeKato requested a review from KuraZuzu October 29, 2024 02:17
@KuraZuzu
Copy link
Contributor

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

@KuraZuzu
Copy link
Contributor

KuraZuzu commented Oct 29, 2024

依存関係のincludeをヘッダファイルへ集約することを推奨します。

以下の変更を加えた状態で、実機の動作確認もできています。
環境:Raspberry Pi 4B+ (4GB)上のUbuntu Server 24.04 (6.8.0-1013-raspi)

rtmouse.c

以下のincludeをrtmouse.hに転記

// 以下のincludeしているヘッダの定義を"rtmouse.h"に移動
#include <linux/cdev.h>
#include <linux/delay.h>
#include <linux/device.h>
#include <linux/errno.h>
#include <linux/fs.h>
#include <linux/i2c.h>
#include <linux/iio/iio.h>
#include <linux/init.h>
#include <linux/io.h>
#include <linux/ioport.h>
#include <linux/kdev_t.h>
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/moduleparam.h>
#include <linux/mutex.h>
#include <linux/sched.h>
#include <linux/slab.h>
#include <linux/spi/spi.h>
#include <linux/stat.h>
#include <linux/timer.h>
#include <linux/types.h>
#include <linux/uaccess.h>
#include <linux/version.h>

rtmouse.h

rtmouse.cから転記

#define RTMOUSE_H

// 依存するincludeファイルの記述を`rtmouse.c`から転記
#include <linux/cdev.h>
#include <linux/delay.h>
#include <linux/device.h>
#include <linux/errno.h>
#include <linux/fs.h>
#include <linux/i2c.h>
#include <linux/iio/iio.h>
#include <linux/init.h>
#include <linux/io.h>
#include <linux/ioport.h>
#include <linux/kdev_t.h>
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/moduleparam.h>
#include <linux/mutex.h>
#include <linux/sched.h>
#include <linux/slab.h>
#include <linux/spi/spi.h>
#include <linux/stat.h>
#include <linux/timer.h>
#include <linux/types.h>
#include <linux/uaccess.h>
#include <linux/version.h>

@KuraZuzu
Copy link
Contributor

rtmouse.cCopyrightの年の更新を推奨します

変更前

/*
 *
 * rtmouse.c
 * Raspberry Pi Mouse device driver
 *
 * Version: 3.3.2
 *
 * Copyright (C) 2015-2021 RT Corporation <shop@rt-net.jp>
 *

変更後

/*
 *
 * rtmouse.c
 * Raspberry Pi Mouse device driver
 *
 * Version: 3.3.2
 *
 * Copyright (C) 2015-2024 RT Corporation <shop@rt-net.jp>
 *

@KuraZuzu
Copy link
Contributor

何点かレビューコメントをしましたのでご確認をお願いします。

@YusukeKato
Copy link
Contributor Author

レビューコメントありがとうございます。
追加で下記の修正を行いました。

  • rtmouse.cからrtmouse.hへヘッダファイルを移動
  • Copyrightを更新
  • rtmouse.hがlintの対象になっていなかったので追加

@YusukeKato
Copy link
Contributor Author

rtmouse.hにライセンスを追加しました。

@KuraZuzu
Copy link
Contributor

修正ありがとうございました。再度動作確認できました。
LGTMです。

@KuraZuzu KuraZuzu merged commit 43532e9 into master Oct 29, 2024
11 checks passed
@KuraZuzu KuraZuzu deleted the refactoring_divide_file branch October 29, 2024 08:57
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