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

Make I2Cscan non-blocking #378

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

kounocom
Copy link
Contributor

Makes I2Cscan non-blocking, resolves #377. I could only test it with one IMU on ESP8266, where it still connected to wifi even when the IMU was not present at all, or on other pins/address. Needs further testing.

@gorbit99 gorbit99 self-requested a review January 17, 2025 13:18
lib/i2cscan/i2cscan.cpp Outdated Show resolved Hide resolved
lib/i2cscan/i2cscan.cpp Outdated Show resolved Hide resolved
lib/i2cscan/i2cscan.cpp Outdated Show resolved Hide resolved
lib/i2cscan/i2cscan.h Outdated Show resolved Hide resolved
lib/i2cscan/i2cscan.cpp Show resolved Hide resolved
Copy link
Contributor

@gorbit99 gorbit99 left a comment

Choose a reason for hiding this comment

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

lgtm

@kounocom
Copy link
Contributor Author

Tested to behave in a non-blocking way on official Slimes too when the IMU is misconfigured, though mine just prints that there's an I2C device on every address and pin, even with main firmware...

@kounocom kounocom marked this pull request as ready for review January 17, 2025 13:57
@kounocom kounocom requested a review from Eirenliel as a code owner January 17, 2025 13:57
@unlogisch04
Copy link
Contributor

Why remove most of the comments?
Why remove port exclude? (LED or Flash might be affected)

@kounocom
Copy link
Contributor Author

portExclude is unused, comments seemed to clutter the code but if that's a dealbreaker I can add them back

@unlogisch04
Copy link
Contributor

I agree that it is a little mess with the comments.
But also the comments are documentation what the code does. Also removing a copyright is not the correct way i believe.

For the portExclude thing, why i did add it:
https://github.com/SlimeVR/SlimeVR-Tracker-ESP/pull/378/files#diff-3d0e70933593b8764e7020c8166d224ede8629400ba68de12ba803d92dace3e4L50
It makes sure that the flash and LED still works after running the i2c scan. (Flash and LED are normal GPIO pins in other configurations). That was the idea about the portExclude.

@kounocom kounocom requested a review from gorbit99 January 19, 2025 06:20
Copy link
Member

@ButterscotchV ButterscotchV left a comment

Choose a reason for hiding this comment

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

Code looks fine, will test in a few minutes, pls wait before merge

@ButterscotchV
Copy link
Member

Code looks fine, will test in a few minutes, pls wait before merge

Works fine, tracker had BNO fuckery but it seems like it functions as intended and I was able to recover my tracker with OTA, no USB necessary 👍

@Eirenliel
Copy link
Member

Please don't merge without me testing it tomorrow or the day after.

Copy link
Member

@Eirenliel Eirenliel left a comment

Choose a reason for hiding this comment

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

Wait for me testing it in one or two days.

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

Successfully merging this pull request may close these issues.

Flashing misconfigured firmware bricks tracker
5 participants