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

sht3x: use periodic report mode #6634

Merged
merged 2 commits into from
Jul 6, 2024

Conversation

nefelim4ag
Copy link
Contributor

SHT3X supports periodic mode, which allows us to completely skip the "write" phase and wait, and just read data in one command.

Also, I corrected reactor pauses to the right numeric order (they are also wrong in htu21.py).

Thanks.

@JamesH1978
Copy link
Collaborator

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

@KevinOConnor
Copy link
Collaborator

Thanks. Happy to commit this when you are ready.

Note, though, that host based pause() commands do not enforce a chip delay. The i2c_write() commands are queued and sent to the mcu for execution - pausing between queuing of commands does not ensure the mcu will have a pause between execution of commands.

There are a variety of ways to fix this. I'd guess the simplest would be to add an i2c_write_wait_ack() to bus.py (that uses self.i2c_write_cmd.send_wait_ack()). I'm not entirely sure what the sht3x chip requirements are though.

-Kevin

@nefelim4ag
Copy link
Contributor Author

nefelim4ag commented Jul 4, 2024

I'm not an experienced embedded developer, so I write pauses according to the datasheet with some additional sane margin "just in case".

In HTU code they all look like x10 of max datasheet values, maybe because reactor pause does not guarantee timings?
So, Initially, without complete understanding, I used the same order of values.
(like 20ms = 0.20, instead of 0.020)

With that said:

  • If you would like, I can add and test wait_ack for i2c here, and after for some bme sensors, because I also have them (in separate PR if their timing differs as here). It looks like wait_ack should not cause issues, only a little overhead.
  • Or, I can return pauses to x10 of max values and leave everything as is.

Thanks

@KevinOConnor KevinOConnor merged commit 248d3db into Klipper3d:master Jul 6, 2024
1 check passed
@KevinOConnor
Copy link
Collaborator

Thanks. I committed this change.

The pause() timing is accurate - I'd guess that other code exaggerates delays to try to account for host delays not easily correlating to mcu delays. If you can make and test fixes for the bme code then that would be great.

-Kevin

@nefelim4ag nefelim4ag deleted the sht3x-periodic branch July 29, 2024 19:32
Misterke pushed a commit to Misterke/klipper that referenced this pull request Nov 1, 2024
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.

3 participants