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

SNS - Changes for IMUs to work #152

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

SNS - Changes for IMUs to work #152

wants to merge 6 commits into from

Conversation

SnickeyX
Copy link
Contributor

@SnickeyX SnickeyX commented Jun 10, 2022

Slight modifications which gets the IMU-20948 to work with our code on the beagleboneblack.

Giving read and write permissions when opening gpio export file to prevent consistent export errors.

@codecov-commenter
Copy link

codecov-commenter commented Jun 10, 2022

Codecov Report

Merging #152 (7152017) into master (516a539) will decrease coverage by 0.03%.
The diff coverage is 11.11%.

@@            Coverage Diff             @@
##           master     #152      +/-   ##
==========================================
- Coverage   38.20%   38.16%   -0.04%     
==========================================
  Files          69       69              
  Lines        3942     3946       +4     
==========================================
  Hits         1506     1506              
- Misses       2436     2440       +4     
Impacted Files Coverage Δ
src/sensors/imu.cpp 49.35% <0.00%> (-0.33%) ⬇️
src/utils/io/spi.cpp 9.52% <16.66%> (-0.29%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 516a539...7152017. Read the comment docs.

src/utils/io/spi.cpp Outdated Show resolved Hide resolved
@SnickeyX SnickeyX requested a review from kshxtij June 11, 2022 09:17
Copy link
Member

@mifrandir mifrandir left a comment

Choose a reason for hiding this comment

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

Small things; good stuff otherwise!

src/sensors/imu.cpp Show resolved Hide resolved
src/utils/io/spi.cpp Show resolved Hide resolved
@@ -105,7 +105,7 @@ Spi::Spi(Logger &log) : spi_fd_(-1), hw_(0), ch_(0), log_(log)
}

// set clock frequency
setClock(Clock::k1MHz);
setClock(Clock::k500KHz);
Copy link
Member

Choose a reason for hiding this comment

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

Is this required? Did we test it? Has it to do with the new IMUs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, yes and yes. Although this just needs to be tested with the sensors PCB but with an individual IMU, this worked.

src/utils/io/spi.cpp Outdated Show resolved Hide resolved
@SnickeyX SnickeyX requested a review from mifrandir June 23, 2022 11:46
@@ -133,7 +133,7 @@ void Gpio::exportGPIO()
// let the kernel know we are using this pin
int fd;
uint32_t len;
fd = open("/sys/class/gpio/export", O_WRONLY);
fd = open("/sys/class/gpio/export", O_RDWR);
Copy link
Member

Choose a reason for hiding this comment

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

You've touched this line now so we need to do this properly.

const auto fd = open(/* ... */);

Similar for len.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants