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

Improved Linux GPIO Driver Using Chardev #2943

Merged
merged 19 commits into from
Oct 14, 2024
Merged

Conversation

LeStarch
Copy link
Collaborator

Related Issue(s)
Has Unit Tests (y/n)
Documentation Included (y/n)

Change Description

Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

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

CodeQL found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.

Copy link
Collaborator

@thomas-bc thomas-bc left a comment

Choose a reason for hiding this comment

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

LGTM! A few comments/recommendations, but not blocking

Drv/LinuxGpioDriver/LinuxGpioDriver.fpp Outdated Show resolved Hide resolved
Drv/LinuxGpioDriver/LinuxGpioDriver.hpp Outdated Show resolved Hide resolved
Copy link
Collaborator

@kevin-f-ortega kevin-f-ortega left a comment

Choose a reason for hiding this comment

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

Looks good, just a few comments

Drv/LinuxGpioDriver/LinuxGpioDriver.cpp Outdated Show resolved Hide resolved
Drv/LinuxGpioDriver/LinuxGpioDriver.cpp Outdated Show resolved Hide resolved
Drv/LinuxGpioDriver/LinuxGpioDriver.cpp Outdated Show resolved Hide resolved
Drv/LinuxGpioDriver/LinuxGpioDriver.cpp Outdated Show resolved Hide resolved
Drv/LinuxGpioDriver/LinuxGpioDriver.cpp Outdated Show resolved Hide resolved
Drv/LinuxGpioDriver/LinuxGpioDriver.cpp Outdated Show resolved Hide resolved
Drv/LinuxGpioDriver/LinuxGpioDriver.cpp Show resolved Hide resolved
Drv/LinuxGpioDriver/LinuxGpioDriver.cpp Outdated Show resolved Hide resolved
Drv/LinuxGpioDriver/LinuxGpioDriver.cpp Show resolved Hide resolved
Drv/LinuxGpioDriver/LinuxGpioDriver.cpp Outdated Show resolved Hide resolved
namespace Drv {


Os::File::Status errno_to_file_status(PlatformIntType errno_input) {

Check notice

Code scanning / CodeQL

Long function without assertion Note

All functions of more than 10 lines should have at least one assertion.
return status;
}

Drv::GpioStatus errno_to_gpio_status(PlatformIntType errno_input) {

Check notice

Code scanning / CodeQL

Long function without assertion Note

All functions of more than 10 lines should have at least one assertion.
return status;
}

U32 configuration_to_handler_flags(Drv::LinuxGpioDriver::GpioConfiguration configuration) {

Check notice

Code scanning / CodeQL

Long function without assertion Note

All functions of more than 10 lines should have at least one assertion.
return flags;
}

U32 configuration_to_event_flags(Drv::LinuxGpioDriver::GpioConfiguration configuration) {

Check notice

Code scanning / CodeQL

Long function without assertion Note

All functions of more than 10 lines should have at least one assertion.
// Handler implementations for user-defined typed input ports
// ----------------------------------------------------------------------

Os::File::Status LinuxGpioDriver ::setupLineHandle(const PlatformIntType chip_descriptor,

Check notice

Code scanning / CodeQL

Long function without assertion Note

All functions of more than 10 lines should have at least one assertion.
return status;
}

Drv::GpioStatus LinuxGpioDriver ::gpioRead_handler(const NATIVE_INT_TYPE portNum, Fw::Logic& state) {

Check notice

Code scanning / CodeQL

Long function without assertion Note

All functions of more than 10 lines should have at least one assertion.
return status;
}

Drv::GpioStatus LinuxGpioDriver ::gpioWrite_handler(const NATIVE_INT_TYPE portNum, const Fw::Logic& state) {

Check notice

Code scanning / CodeQL

Long function without assertion Note

All functions of more than 10 lines should have at least one assertion.
return status;
}

void LinuxGpioDriver ::pollLoop() {

Check notice

Code scanning / CodeQL

Long function without assertion Note

All functions of more than 10 lines should have at least one assertion.
// Construction, initialization, and destruction
// ----------------------------------------------------------------------

LinuxGpioDriver ::LinuxGpioDriver(const char* const compName) : LinuxGpioDriverComponentBase(compName) {}

Check notice

Code scanning / CodeQL

More than one statement per line Note

This line contains 2 statements; only one is allowed.

LinuxGpioDriver ::LinuxGpioDriver(const char* const compName) : LinuxGpioDriverComponentBase(compName) {}

Drv::GpioStatus LinuxGpioDriver ::start(const FwSizeType priority,

Check notice

Code scanning / CodeQL

Long function without assertion Note

All functions of more than 10 lines should have at least one assertion.
return status;
}

Os::File::Status LinuxGpioDriver ::open(const char* device,

Check notice

Code scanning / CodeQL

Function too long Note

open has too many lines (73, while 60 are allowed).
@LeStarch LeStarch merged commit b9ea1fd into nasa:devel Oct 14, 2024
35 checks passed
@LeStarch LeStarch added the Update Instructions Needed Need to add instructions in the release notes for updates. label Oct 14, 2024
thomas-bc added a commit to fprime-community/fprime-workshop-led-blinker that referenced this pull request Oct 15, 2024
* Fixing LedBlinker workshop for new GPIO driver

* Fixing UT breakage

---------

Co-authored-by: Michael D Starch <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Update Instructions Needed Need to add instructions in the release notes for updates.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants