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

[develop] Fix pimoroni trackball read address. #13810

Merged
merged 1 commit into from
Jul 31, 2021
Merged

Conversation

daskygit
Copy link
Member

Description

Changes the read address, I was unable to read trackball status without this change. Any queries would end up with I2C_STATUS_ERROR.

Works fine with the change, tested on a promicro and pimoroni trackball.

Types of Changes

  • Core
  • Bugfix

Checklist

  • My code follows the code style of this project: C, Python
  • I have read the PR Checklist document and have made the appropriate changes.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • I have tested the changes and verified that they work and don't break anything (as well as I can manage).

@github-actions github-actions bot added the core label Jul 31, 2021
@fauxpark fauxpark requested a review from a team July 31, 2021 07:03
@drashna drashna merged commit aeb2524 into qmk:develop Jul 31, 2021
@zvecr
Copy link
Member

zvecr commented Jul 31, 2021

I think I would be expecting something more like....

diff --git a/drivers/sensors/pimoroni_trackball.c b/drivers/sensors/pimoroni_trackball.c
index c0ac644f7..d79a3e815 100644
--- a/drivers/sensors/pimoroni_trackball.c
+++ b/drivers/sensors/pimoroni_trackball.c
@@ -17,15 +17,9 @@
 #include "pimoroni_trackball.h"
 #include "i2c_master.h"
 
-static uint8_t scrolling      = 0;
-static int16_t x_offset       = 0;
-static int16_t y_offset       = 0;
-static int16_t h_offset       = 0;
-static int16_t v_offset       = 0;
-static float   precisionSpeed = 1;
-
-static uint16_t i2c_timeout_timer;
-
+#ifndef TRACKBALL_ADDRESS
+#    define TRACKBALL_ADDRESS 0x0A
+#endif
 #ifndef PIMORONI_I2C_TIMEOUT
 #    define PIMORONI_I2C_TIMEOUT 100
 #endif
@@ -36,9 +30,22 @@ static uint16_t i2c_timeout_timer;
 #    define MOUSE_DEBOUNCE 5
 #endif
 
+#define TRACKBALL_SHIFTED_ADDRESS (TRACKBALL_ADDRESS << 1)
+#define TRACKBALL_TRANSMIT_WRITE  ((TRACKBALL_ADDRESS << 1) | I2C_WRITE)
+#define TRACKBALLL_TRANSMIT_READ  ((TRACKBALL_ADDRESS << 1) | I2C_READ)
+
+static uint8_t scrolling      = 0;
+static int16_t x_offset       = 0;
+static int16_t y_offset       = 0;
+static int16_t h_offset       = 0;
+static int16_t v_offset       = 0;
+static float   precisionSpeed = 1;
+
+static uint16_t i2c_timeout_timer;
+
 void trackball_set_rgbw(uint8_t red, uint8_t green, uint8_t blue, uint8_t white) {
     uint8_t data[] = {0x00, red, green, blue, white};
-    i2c_transmit(TRACKBALL_WRITE, data, sizeof(data), PIMORONI_I2C_TIMEOUT);
+    i2c_transmit(TRACKBALL_TRANSMIT_WRITE, data, sizeof(data), PIMORONI_I2C_TIMEOUT);
 }
 
 int16_t mouse_offset(uint8_t positive, uint8_t negative, int16_t scale) {
@@ -80,7 +87,7 @@ void pointing_device_task(void) {
     static uint16_t debounce_timer;
     uint8_t         state[5] = {};
     if (timer_elapsed(i2c_timeout_timer) > I2C_WAITCHECK) {
-        if (i2c_readReg(TRACKBALL_READ, 0x04, state, 5, PIMORONI_I2C_TIMEOUT) == I2C_STATUS_SUCCESS) {
+        if (i2c_readReg(TRACKBALL_SHIFTED_ADDRESS, 0x04, state, 5, PIMORONI_I2C_TIMEOUT) == I2C_STATUS_SUCCESS) {
             if (!state[4] && !debounce) {
                 if (scrolling) {
 #ifdef PIMORONI_TRACKBALL_INVERT_X
diff --git a/drivers/sensors/pimoroni_trackball.h b/drivers/sensors/pimoroni_trackball.h
index a30fb0bb8..ad4393eb4 100644
--- a/drivers/sensors/pimoroni_trackball.h
+++ b/drivers/sensors/pimoroni_trackball.h
@@ -19,12 +19,6 @@
 #include "quantum.h"
 #include "pointing_device.h"
 
-#ifndef TRACKBALL_ADDRESS
-#    define TRACKBALL_ADDRESS 0x0A
-#endif
-#define TRACKBALL_WRITE ((TRACKBALL_ADDRESS << 1) | I2C_WRITE)
-#define TRACKBALL_READ ((TRACKBALL_ADDRESS << 1) | I2C_READ)
-
 void trackball_set_rgbw(uint8_t red, uint8_t green, uint8_t blue, uint8_t white);
 void trackball_check_click(bool pressed, report_mouse_t *mouse);
 void trackball_register_button(bool pressed, enum mouse_buttons button);

@daskygit
Copy link
Member Author

I think I would be expecting something more like....

diff --git a/drivers/sensors/pimoroni_trackball.c b/drivers/sensors/pimoroni_trackball.c
index c0ac644f7..d79a3e815 100644
--- a/drivers/sensors/pimoroni_trackball.c
+++ b/drivers/sensors/pimoroni_trackball.c
@@ -17,15 +17,9 @@
 #include "pimoroni_trackball.h"
 #include "i2c_master.h"
 
-static uint8_t scrolling      = 0;
-static int16_t x_offset       = 0;
-static int16_t y_offset       = 0;
-static int16_t h_offset       = 0;
-static int16_t v_offset       = 0;
-static float   precisionSpeed = 1;
-
-static uint16_t i2c_timeout_timer;
-
+#ifndef TRACKBALL_ADDRESS
+#    define TRACKBALL_ADDRESS 0x0A
+#endif
 #ifndef PIMORONI_I2C_TIMEOUT
 #    define PIMORONI_I2C_TIMEOUT 100
 #endif
@@ -36,9 +30,22 @@ static uint16_t i2c_timeout_timer;
 #    define MOUSE_DEBOUNCE 5
 #endif
 
+#define TRACKBALL_SHIFTED_ADDRESS (TRACKBALL_ADDRESS << 1)
+#define TRACKBALL_TRANSMIT_WRITE  ((TRACKBALL_ADDRESS << 1) | I2C_WRITE)
+#define TRACKBALLL_TRANSMIT_READ  ((TRACKBALL_ADDRESS << 1) | I2C_READ)
+
+static uint8_t scrolling      = 0;
+static int16_t x_offset       = 0;
+static int16_t y_offset       = 0;
+static int16_t h_offset       = 0;
+static int16_t v_offset       = 0;
+static float   precisionSpeed = 1;
+
+static uint16_t i2c_timeout_timer;
+
 void trackball_set_rgbw(uint8_t red, uint8_t green, uint8_t blue, uint8_t white) {
     uint8_t data[] = {0x00, red, green, blue, white};
-    i2c_transmit(TRACKBALL_WRITE, data, sizeof(data), PIMORONI_I2C_TIMEOUT);
+    i2c_transmit(TRACKBALL_TRANSMIT_WRITE, data, sizeof(data), PIMORONI_I2C_TIMEOUT);
 }
 
 int16_t mouse_offset(uint8_t positive, uint8_t negative, int16_t scale) {
@@ -80,7 +87,7 @@ void pointing_device_task(void) {
     static uint16_t debounce_timer;
     uint8_t         state[5] = {};
     if (timer_elapsed(i2c_timeout_timer) > I2C_WAITCHECK) {
-        if (i2c_readReg(TRACKBALL_READ, 0x04, state, 5, PIMORONI_I2C_TIMEOUT) == I2C_STATUS_SUCCESS) {
+        if (i2c_readReg(TRACKBALL_SHIFTED_ADDRESS, 0x04, state, 5, PIMORONI_I2C_TIMEOUT) == I2C_STATUS_SUCCESS) {
             if (!state[4] && !debounce) {
                 if (scrolling) {
 #ifdef PIMORONI_TRACKBALL_INVERT_X
diff --git a/drivers/sensors/pimoroni_trackball.h b/drivers/sensors/pimoroni_trackball.h
index a30fb0bb8..ad4393eb4 100644
--- a/drivers/sensors/pimoroni_trackball.h
+++ b/drivers/sensors/pimoroni_trackball.h
@@ -19,12 +19,6 @@
 #include "quantum.h"
 #include "pointing_device.h"
 
-#ifndef TRACKBALL_ADDRESS
-#    define TRACKBALL_ADDRESS 0x0A
-#endif
-#define TRACKBALL_WRITE ((TRACKBALL_ADDRESS << 1) | I2C_WRITE)
-#define TRACKBALL_READ ((TRACKBALL_ADDRESS << 1) | I2C_READ)
-
 void trackball_set_rgbw(uint8_t red, uint8_t green, uint8_t blue, uint8_t white);
 void trackball_check_click(bool pressed, report_mouse_t *mouse);
 void trackball_register_button(bool pressed, enum mouse_buttons button);

I'm currently playing around with a modified driver that appears to be faster, which I'm hoping to pull request. I'll make sure to implement the suggestions. Although it only uses readReg and writeReg so only the shifted address is required.

@daskygit daskygit deleted the fix/pt branch August 2, 2021 22:52
nhongooi pushed a commit to nhongooi/qmk_firmware that referenced this pull request Dec 5, 2021
BorisTestov pushed a commit to BorisTestov/qmk_firmware that referenced this pull request May 23, 2024
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