2 year old adxl345* bug with exact commit identified (*corrected from rp2040)

General Discussion
1

I tried bringing this up a few times in the past, but since a new release is on the horizon and I saw a post from Kevin that he wanted bug reports, I’m going to try again here.

Yes, this is the wrong forum. Github points people trying to create issues here. The “Developers” forum here specifically states it’s not for bug reports when you start a new draft.
I don’t see a more appropriate forum here for bug reports… please point me in the right direction if I’m missing something obvious.

In the commit below commit, you can see that the check to see if the Pico is initialized is deleted.

        if not self.is_initialized():
            self.initialize()

The result of this deletion is the first time you try and access the chip (following the documentation) using ACCELEROMETER_QUERY, it fails with an error.
It is initialized after this first access, so all subsequent accesses succeed.
This is 100% repeatable including in the most recent code (I just re-tested it multiple times).
This does not happen with the release immediately preceding the above commit.
If you’d like to see for yourself, that commit is:

Note that I am not the only one seeing this bug. I still get replies to my posts on Reddit from 2 years ago thanking me for the workaround.
My thread:
https://www.reddit.com/r/klippers/comments/owqvo2/raspberry_pi_pico_as_a_secondary_mcu_for/
Someone else’s thread that I commented in:
https://www.reddit.com/r/klippers/comments/tr0r8c/invalid_adxl345_id_got_0_vs_e5/
In practice, this is what it looks like from a terminal when you run it twice back to back (it’s not bad wiring):

Send: ACCELEROMETER_QUERY
Recv: // Invalid adxl345 id (got 0 vs e5).
Recv: // This is generally indicative of connection problems
Recv: // (e.g. faulty wiring) or a faulty adxl345 chip.
Recv: !! Invalid adxl345 id (got 0 vs e5).
Recv: ok
Send: ACCELEROMETER_QUERY
Recv: // accelerometer values (x, y, z): 5329.482782, 52630.760713, -6217.729913
Recv: ok

Again, the first query after a firmware restart always fails, and all subsequent queries succeed.
Another user confirmed it here (in the bottom of his post, the summary doesn’t show it):

As did another user further down the discussion.

2

Thanks. I’m not sure if you are reporting an issue, or suggesting a fix, however.

I’d guess something in the low-level rp2040 SPI initialization isn’t getting things 100% correct on the first try, but things stabilize after that. I’d guess the fix is somewhere deep in the rp2040 code, but no one has had a chance to look at it.

If you think you have a fix, could you specify what you believe the fix is?

Thanks again,
-Kevin

3

My apologies for the think-o in the subject (I was quite tired when I wrote that).
It’s a bug in adxl345.py, not a bug in the rp2040 code.

I’m not sure I’m qualified to code a solution as I don’t understand the reasoning for the code change, but I’m certain I narrowed it to the exact patch (which is quite small).

The code to check to see if the chip is initialized (and initialize it if it is not) was removed from lines 266 & 267, and the functions themselves were deleted from lines 205-217.
It appears to initialize the chip using self.set_reg(REG_POWER_CTL, 0x00) in the deleted function.

What was added (253-258) attempts to access the chip before it is initialized (resulting in the error message). The initialization then appears to happen at line 272(left column) / 263(right column) using self.set_reg(REG_POWER_CTL, 0x00, minclock=clock).
This would explain why it always works after the first access, but never on the first access (after a firmware reset).

4

This sounds like a bug in the rp2040 code. Other chips don’t have this issue (none of my printers have this issue).

The adxl345 chip does not need to be initialized in order to read its ID register. So, if the ID register isn’t reporting a valid ID, it’s not due to lack of initialization.

It’s possible to retry the ID read on a failure - that’s probably why this used to work on rp2040, but the real fix is to figure out what is happening on the low-level SPI rp2040 code.

For example:

--- a/klippy/extras/adxl345.py
+++ b/klippy/extras/adxl345.py
@@ -373,11 +373,13 @@ class ADXL345:
         # noise or wrong signal as a correctly initialized device
         dev_id = self.read_reg(REG_DEVID)
         if dev_id != ADXL345_DEV_ID:
-            raise self.printer.command_error(
-                "Invalid adxl345 id (got %x vs %x).\n"
-                "This is generally indicative of connection problems\n"
-                "(e.g. faulty wiring) or a faulty adxl345 chip."
-                % (dev_id, ADXL345_DEV_ID))
+            dev_id = self.read_reg(REG_DEVID)
+            if dev_id != ADXL345_DEV_ID:
+                raise self.printer.command_error(
+                    "Invalid adxl345 id (got %x vs %x).\n"
+                    "This is generally indicative of connection problems\n"
+                    "(e.g. faulty wiring) or a faulty adxl345 chip."
+                    % (dev_id, ADXL345_DEV_ID))
         # Setup chip in requested query rate
         self.set_reg(REG_POWER_CTL, 0x00)
         self.set_reg(REG_DATA_FORMAT, 0x0B)

Cheers,
-Kevin

5

I have tested a lot with various ADXLs and on various platforms, like OPis, RPis, BTT CB1, RP2040 etc.
I can confirm that this SPI initialization problem is only present on the RP2040 or at least I did not encounter it on any other platform.

1 Like
6

That’s both surprising and confusing, and makes me wonder why the (removed) initialization code was in there in the first place (since it actually worked). It looks like what you’re saying has been confirmed by @Sineos

It looks like the initialization code was added by @dmbutyugin in this commit:

Maybe he could explain why it was there.

@Sineos, thank you for going through the trouble of testing so many different ways!

7

FWIW, this effect seems only to happen when connecting the ADXL via hardware SPI. On software SPI it does not for me.

1 Like
8

That’s an excellent observation and further reinforces Kevin’s assertion that it’s actually an rp2040 bug.
I’d completely forgotten that pure-software SPI was a thing (it’s been years since I figured out and wrote up how to wire and configure the Pico in my reddit post, mentioned above).

I’m just hoping at this point that @dmbutyugin can assist, since he’s the one that introduced the code that prevented this error.

9

This topic was automatically closed 60 days after the last reply. New replies are no longer allowed.