klipper icon indicating copy to clipboard operation
klipper copied to clipboard

Update LIS2DW example to match current BTT documentation

Open buzztiaan opened this issue 2 years ago • 9 comments

As found while testing my own BTT S2DW board, the klipper doc example was using wrong pins (and method?).

Signed-off-by: Bastiaan van den Berg [email protected]

buzztiaan avatar Dec 17 '23 12:12 buzztiaan

Thank you for submitting a PR, pleas refer to point 3 in "What to expect in a review" in https://github.com/Klipper3d/klipper/blob/master/docs/CONTRIBUTING.md and provide a signed off by line.

Thanks James

JamesH1978 avatar Dec 17 '23 13:12 JamesH1978

I'm not sure if this is the right thing to do.

You have two ways of connecting a SPI IMU like the LIS2DW or ADXL3xx:

Hardware SPI:

[...]
cs_pin: lis:gpio1
spi_bus: spi0a

Software SPI:

[...]
cs_pin: lis:gpio9
spi_software_sclk_pin: lis:gpio10
spi_software_mosi_pin: lis:gpio11
spi_software_miso_pin: lis:gpio8

The S2DW is a combination of a RP2040 MCU and a LIS2DW and BTT (in this very product) choose to wire it in software SPI due to an issue with hardware SPI that is also mentioned here: https://www.klipper3d.org/Measuring_Resonances.html#measuring-the-resonances grafik

Generally I'd prefer to use hardware SPI but with this combination it might be preferable to use software SPI for the casual user. If we go by this, then the entire chapter should be reworked and software SPI be recommended for the RP2040 / SPI combination.

Sineos avatar Dec 17 '23 13:12 Sineos

The 'SW instead of HW' is just copied from BTT's example. As the original config blurb came from them, it seemed wise to update it to mirror.

Beside the 'SW instead' , its also using different pins. The specified pins aren't connected to the S2DW on this specific product. Also i haven't seen any other products with the LIS2DW chip included.

buzztiaan avatar Dec 18 '23 09:12 buzztiaan

The documentation is meant to explain the principle and not be tailored to a single product.

There is a similar product from Fly which is using different pins.

And still the hardware SPI is preferable (hoping the SPI issue can be resolved).

Sineos avatar Dec 18 '23 12:12 Sineos

The linked fly docs say they DO use the same pins as my pr, plus also SW spi

On Mon, 18 Dec 2023, 13:40 Sineos, @.***> wrote:

The documentation is meant to explain the principle and not be tailored to a single product.

There is a similar product from Fly https://mellow.klipper.cn/?spm=a2g0o.detail.1000023.1.c5665Pp05Pp0He#/advanced/usb_lis2dw which is using different pins.

And still the hardware SPI is preferable (hoping the SPI issue can be resolved).

— Reply to this email directly, view it on GitHub https://github.com/Klipper3d/klipper/pull/6431#issuecomment-1860393890, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAHFSMBSSMA7P5QWGEFHTJLYKA2UFAVCNFSM6AAAAABAYKAGF6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNRQGM4TGOBZGA . You are receiving this because you authored the thread.Message ID: @.***>

buzztiaan avatar Dec 18 '23 14:12 buzztiaan

Fly:

[lis2dw]
cs_pin: LIS:gpio9
spi_software_sclk_pin: LIS:gpio10
spi_software_mosi_pin: LIS:gpio11
spi_software_miso_pin: LIS:gpio12

BTT:

[lis2dw]
cs_pin: lis:gpio9
spi_software_sclk_pin: lis:gpio10
spi_software_mosi_pin: lis:gpio11
spi_software_miso_pin: lis:gpio8

¯\_(ツ)_/¯

Sineos avatar Dec 18 '23 16:12 Sineos

Oh, missed the miso pin ;) either way, its a lot closer :D hahaha Maybe its more suitable for a example-config then?

buzztiaan avatar Dec 18 '23 18:12 buzztiaan

In principle, I agree with you, but then again, the entire documentation with respect to this would have to be reworked.

ADXL3xx Situation:

  • Stand alone break-out board are existing
  • For these boards the documentation is correct as is
  • For the ADXL + RD2040 the same issue applies: SPI will error out on the first attempt but work for the subsequent ones
  • For the commercial made boards that integrate an ADXL with a RD2040, e.g. https://github.com/bigtreetech/ADXL345/blob/master/Firmware/sample-bigtreetech-adxl345-v2.0.cfg, the documentation is as "wrong" as for the LIS2DW since the manufacturers work around the SPI issue by using software SPI

LIS2DW situation:

  • Except https://wiki.dfrobot.com/Gravity_I2C_LIS2DW12_Triple_Axis_Accelerometer_SKU_SEN0409 I do not know any IMU break-out boards with this chip and the DFrobot one is I2C and not SPI
  • The documentation is correct per se and for sure the manufacturers like BTT or Fly would have chosen hardware SPI if there was not this SPI issue
  • The KUSBA-Pro uses hardware SPI, but this board is based on a STM32F042 and as such not affected by the SPI issue

Unfortunately, there is no one-solution-fits-all and running behind the potential pin and wiring scenarios will not really take you anywhere.

Sineos avatar Dec 18 '23 20:12 Sineos

Thank you for your contribution to Klipper. Unfortunately, a reviewer has not assigned themselves to this GitHub Pull Request. All Pull Requests are reviewed before merging, and a reviewer will need to volunteer. Further information is available at: https://www.klipper3d.org/CONTRIBUTING.html

There are some steps that you can take now:

  1. Perform a self-review of your Pull Request by following the steps at: https://www.klipper3d.org/CONTRIBUTING.html#what-to-expect-in-a-review If you have completed a self-review, be sure to state the results of that self-review explicitly in the Pull Request comments. A reviewer is more likely to participate if the bulk of a review has already been completed.
  2. Consider opening a topic on the Klipper Discourse server to discuss this work. The Discourse server is a good place to discuss development ideas and to engage users interested in testing. Reviewers are more likely to prioritize Pull Requests with an active community of users.
  3. Consider helping out reviewers by reviewing other Klipper Pull Requests. Taking the time to perform a careful and detailed review of others work is appreciated. Regular contributors are more likely to prioritize the contributions of other regular contributors.

Unfortunately, if a reviewer does not assign themselves to this GitHub Pull Request then it will be automatically closed. If this happens, then it is a good idea to move further discussion to the Klipper Discourse server. Reviewers can reach out on that forum to let you know if they are interested and when they are available.

Best regards, ~ Your friendly GitIssueBot

PS: I'm just an automated script, not a human being.

github-actions[bot] avatar Jan 02 '24 00:01 github-actions[bot]