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

Issue #7 Correction / If SHUTDOWN (WAKE pin) feature is not needed, the pin can be leave unconnected Issue #9 #8

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

Conversation

rtek1000
Copy link

#7

@rtek1000
Copy link
Author

If SHUTDOWN (WAKE pin) feature is not needed, the pin can be leave unconnected

#9

@rtek1000 rtek1000 changed the title Issue #7 Correction Issue #7 Correction / If SHUTDOWN (WAKE pin) feature is not needed, the pin can be leave unconnected Issue #9 Correction Feb 28, 2024
@rtek1000 rtek1000 changed the title Issue #7 Correction / If SHUTDOWN (WAKE pin) feature is not needed, the pin can be leave unconnected Issue #9 Correction Issue #7 Correction / If SHUTDOWN (WAKE pin) feature is not needed, the pin can be leave unconnected Issue #9 Feb 28, 2024
Copy link
Owner

@Skallwar Skallwar left a comment

Choose a reason for hiding this comment

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

Thanks for you PR 🔥
I can't test it currently so I trust you on this :D
I will try to make a release out of it and if it break something for someone, the old version will still be there just in case

@@ -77,20 +90,14 @@ void GSL1680::clear_reg()
uint8_t DATA[4] = {0x88, 0x01, 0x04, 0x00};
uint8_t TIMER[4] = {20, 5, 5, 20};

Wire.beginTransmission(I2CADDR);
uint8_t _data[1];
Copy link
Owner

Choose a reason for hiding this comment

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

Instead of an array of 1, just make it an uint8_t


int i;
for (i = 0; i < 4; ++i) {
Wire.write(REG[i]);
Wire.write(DATA[i]);
delay(TIMER[i]);
Copy link
Owner

Choose a reason for hiding this comment

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

No need for a delay here? I don't have the hardware anymore but I remember spending a lot of time figuring out the timings, it wouldn't work otherwise

@@ -99,24 +106,19 @@ void GSL1680::reset()
uint8_t DATA[2] = {0x88, 0x04};
uint8_t TIMER[2] = {20, 10};

Wire.beginTransmission(I2CADDR);
uint8_t _data[1];
Copy link
Owner

Choose a reason for hiding this comment

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

Same here


int i;
for (i = 0; i < 2; ++i) {
Wire.write(REG[i]);
Wire.write(DATA[i]);
delay(TIMER[i]);
Copy link
Owner

Choose a reason for hiding this comment

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

Same here, no more delay?

Comment on lines +189 to +196
if(n == 5) {
SERIAL_ERROR.println("i2c write error: timeout");
} else {
SERIAL_ERROR.print("i2c write error: ");
SERIAL_ERROR.print(n);
SERIAL_ERROR.print(" ");
SERIAL_ERROR.println(DATA_REG, HEX);
}
Copy link
Owner

Choose a reason for hiding this comment

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

This block of code is in 2 places, could you make a function out of it?

README.md Outdated
Comment on lines 16 to 17
[Tested with ESP32 DEV KIT V1]

Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
[Tested with ESP32 DEV KIT V1]
## Test hardware
- Arduino Mega 2560 Rev3
- ESP32 DEV KIT V1

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.

None yet

2 participants