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

lis2dw12: Add support for lis2dw12 accelerometer #6312

Merged
merged 3 commits into from
Aug 21, 2023

Conversation

bigtreetech
Copy link
Contributor

lis2dw12 is an accelerometer from STMicroelectronics
With better performance than the ADXL345 according to the datasheet.

@bigtreetech bigtreetech force-pushed the lis2dw branch 2 times, most recently from f9dd113 to eb53991 Compare August 7, 2023 08:43
@bigtreetech
Copy link
Contributor Author

This Build test / build (pull_request) error is Klipper image is too large.
Using Optional features (to reduce code size) locally to disable other features and enable Support external sensor devices can successfully build

@KevinOConnor
Copy link
Collaborator

Thanks. In general it seems fine to me. I have a few comments:

  1. The build tests need to pass - try adding WANT_DISPLAYS=n and/or WANT_SENSORS=n to test/configs/ar100.config to see if that fixes the issue. (Or perhaps @eliasbakken has a suggestion here.)
  2. The new klippy/extras/lis2dw.py and src/sensor_lis2dw.c files need a copyright statement at the top of the file. If you don't want to add your copywrite, you can copy the copyright statements from klippy/extras/adxl345.py and src/sensor_adxl345.c respectively.
  3. The docs/Config_Reference.md file should be updated with [lis2dw].

Also, @dmbutyugin do you have any comments on this PR?

-Kevin

@KevinOConnor KevinOConnor self-assigned this Aug 9, 2023
@eliasbakken
Copy link
Contributor

I think WANT_DISPLAYS is already deselected. Deselecting WANT_SENSORS would also remove adxl345, which some people might be using with ar100. Is it an option to increase the granularity a bit? Either that or remove this new accelerometer by using the "HAVE_LIMITED_CODE_SIZE" setting to make the ar100 pass.

@bigtreetech
Copy link
Contributor Author

Thanks. In general it seems fine to me. I have a few comments:

  1. The build tests need to pass - try adding WANT_DISPLAYS=n and/or WANT_SENSORS=n to test/configs/ar100.config to see if that fixes the issue. (Or perhaps @eliasbakken has a suggestion here.)
  2. The new klippy/extras/lis2dw.py and src/sensor_lis2dw.c files need a copyright statement at the top of the file. If you don't want to add your copywrite, you can copy the copyright statements from klippy/extras/adxl345.py and src/sensor_adxl345.c respectively.
  3. The docs/Config_Reference.md file should be updated with [lis2dw].

Also, @dmbutyugin do you have any comments on this PR?

-Kevin

  1. According to the suggestion of @eliasbakken, the configuration of WANT_LIS2DW has been added. And now all build tests can pass.
  2. The copyright has been added as required.
  3. The instructions of how to configure LIS2DW has been added in docs/Config_Reference.md.

@KevinOConnor
Copy link
Collaborator

Thanks. It looks fine to me. I'll give a couple of days to see if there are additional comments - otherwise I'll look to commit.

Cheers,
-Kevin

Copy link
Collaborator

@dmbutyugin dmbutyugin left a comment

Choose a reason for hiding this comment

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

Thanks. I had just a few comments. I didn't test the accelerometer though, so I cannot comment whether and how it works either in the current state or with the suggested mode change. Besides that, I don't have any major feedback for the support of this accelerometer.

klippy/extras/lis2dw.py Outdated Show resolved Hide resolved
klippy/extras/lis2dw.py Outdated Show resolved Hide resolved
bigtreetech added 3 commits August 18, 2023 09:02
lis2dw12 is an accelerometer from STMicroelectronics(https://www.st.com/resource/en/datasheet/lis2dw12.pdf)
With better performance than the ADXL345 according to the datasheet.

Signed-off-by: XM.Zhou from BigTreeTech [email protected]
Signed-off-by: Alan.Ma from BigTreeTech [email protected]
Signed-off-by: Alan.Ma from BigTreeTech [email protected]
@KevinOConnor KevinOConnor merged commit 5f990f9 into Klipper3d:master Aug 21, 2023
1 check passed
@KevinOConnor
Copy link
Collaborator

Thanks.

-Kevin

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.

4 participants