ldc1612: enable frequency div to reduce noise #7160
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Collaborator
|
master (e60fe3d): nefelim4ag-btt-eddy-reduce-noise (204eb8d): This is a test rig (the eddy sensor just ziptied to the side of the toolhead), so don't conclude anything serious from these results. Cheers, |
Collaborator
|
FWIW, the reduction on "noise" is repeatable in my test rig. I ran the test 3 times in a row with: and got on nefelim4ag-btt-eddy-reduce-noise (204eb8d): And then I returned to master (e60fe3d) and got: -Kevin |
|
[edit] Mine is a BTT Eddy Coil. On master: On 204eb8d ldc1612: enable frequency div to reduce noise: |
Collaborator
Author
|
Cool! Thanks. I honestly don't know why it works.
Btw, if someone have better wording I'm not against it :D Also, I don't like how code looks with divs here. It is just placed in a way, that introduce minimal conflicts with all other PRs. -Timofey |
Collaborator
|
Nice find!
What frequency is your clock input to the ldc1612 and what frequencies do you find are reported by the sensor after calibration?
How about something like the following? --- a/klippy/extras/ldc1612.py
+++ b/klippy/extras/ldc1612.py
@@ -3,7 +3,7 @@
# Copyright (C) 2020-2024 Kevin O'Connor <kevin@koconnor.net>
#
# This file may be distributed under the terms of the GNU GPLv3 license.
-import logging
+import math, logging
from . import bus, bulk_sensor
MIN_MSG_TIME = 0.100
@@ -13,6 +13,7 @@ BATCH_UPDATES = 0.100
LDC1612_ADDR = 0x2a
DEFAULT_LDC1612_FREQ = 12000000
+MAX_SENSOR_FREQ = 4000000.
SETTLETIME = 0.005
DRIVECUR = 15
DEGLITCH = 0x05 # 10 Mhz
@@ -87,8 +88,10 @@ class LDC1612:
self.oid = oid = mcu.create_oid()
self.query_ldc1612_cmd = None
self.ldc1612_setup_home_cmd = self.query_ldc1612_home_state_cmd = None
- self.frequency = config.getint("frequency", DEFAULT_LDC1612_FREQ,
- 2000000, 40000000)
+ self.clock_freq = config.getint("frequency", DEFAULT_LDC1612_FREQ,
+ 2000000, 40000000)
+ self.sensor_div = int(math.ceil(4.*MAX_SENSOR_FREQ / self.clock_freq))
+ self.freq_conv = float(self.clock_freq * sensor_div) / (1<<28)
if config.get('intb_pin', None) is not None:
ppins = config.get_printer().lookup_object("pins")
pin_params = ppins.lookup_pin(config.get('intb_pin'))
@@ -143,7 +146,7 @@ class LDC1612:
def setup_home(self, print_time, trigger_freq,
trsync_oid, hit_reason, err_reason):
clock = self.mcu.print_time_to_clock(print_time)
- tfreq = int(trigger_freq * (1<<28) / float(self.frequency) + 0.5)
+ tfreq = int(trigger_freq / self.freq_conv + 0.5)
self.ldc1612_setup_home_cmd.send(
[self.oid, clock, tfreq, trsync_oid, hit_reason, err_reason])
def clear_home(self):
@@ -155,7 +158,7 @@ class LDC1612:
return self.mcu.clock_to_print_time(tclock)
# Measurement decoding
def _convert_samples(self, samples):
- freq_conv = float(self.frequency) / (1<<28)
+ freq_conv = self.freq_conv
count = 0
for ptime, val in samples:
mv = val & 0x0fffffff
@@ -176,11 +179,11 @@ class LDC1612:
"(e.g. faulty wiring) or a faulty ldc1612 chip."
% (manuf_id, dev_id, LDC1612_MANUF_ID, LDC1612_DEV_ID))
# Setup chip in requested query rate
- rcount0 = self.frequency / (16. * self.data_rate)
+ rcount0 = self.clock_freq / (16. * self.data_rate)
self.set_reg(REG_RCOUNT0, int(rcount0 + 0.5))
self.set_reg(REG_OFFSET0, 0)
- self.set_reg(REG_SETTLECOUNT0, int(SETTLETIME*self.frequency/16. + .5))
- self.set_reg(REG_CLOCK_DIVIDERS0, (1 << 12) | 1)
+ self.set_reg(REG_SETTLECOUNT0, int(SETTLETIME*self.clock_freq/16. + .5))
+ self.set_reg(REG_CLOCK_DIVIDERS0, (self.sensor_div << 12) | 1)
self.set_reg(REG_ERROR_CONFIG, (0x1f << 11) | 1)
self.set_reg(REG_MUX_CONFIG, 0x0208 | DEGLITCH)
self.set_reg(REG_CONFIG, 0x001 | (1<<12) | (1<<10) | (1<<9))-Kevin EDIT: Update patch to also take into account (1<<28). |
Collaborator
Author
Clock input is 24MHz, Coil frequencies are similiar to Eddy's: And this is from Mellow SHT36v3 (I have motan dump data):
I also want to adjust the frequency, this is one of the solutions here. Ah, you have already incorporated something like this. About division enable logic, IDK. In perfect scenario we can try to get the frequency during the current calibration. Cheers, |
nefelim4ag
force-pushed
the
btt-eddy-reduce-noise
branch
2 times, most recently
from
a6dee04 to
b9ff94b
Compare
Collaborator
|
Thanks. I'll commit this when you are ready.
Ah, okay, so some known sensors go as high as 4.5Mhz then.
Agreed; it isn't worth spending a lot of effort on. One minor advantage of: MAX_SENSOR_FREQ = 5000000.
...
self.sensor_div = int(math.ceil(4.*MAX_SENSOR_FREQ / self.clock_freq))is that it hints at why we're doing this (spec says "VALID ƒINx RANGE: < ƒREFx /4"). It's not a big deal either way though. Cheers, |
Collaborator
|
As a slight tangent, if I'm reading table 43 of the spec correctly, the SHT36v3 should be setting FREF_DIVIDER0=2 (as 40Mhz is above the 35Mhz max Fref in single channel mode). self.ref_clock_div = int(math.ceil(nom_clock_freq/35000000.))
self.clock_freq = nom_clock_freq // self.ref_clock_div? -Kevin |
Collaborator
Author
I agree here. From my overall experience with LDC, I think we can add LDC_VERIFY_SENSOR something.
This is correct, it is required by the datasheet, but from the data that I have seen, it seems to work fine 3 Also, I did some local testing before with the divider and internal clock reference source: At least for my internal clock of ~46MHz it changes nothing4. That said, I asked the person who has SHT36v3 on hands (and whom data I use), so we can collect some data before/after the divider: -Timofey Patch updated with the comment. Footnotes
|
BTT Eddy uses 12MHz clock in frequency. Coil is oscillating at 3+MHz. Which is out of spec for LDC1612 sensors. Division of coil frequency seems to reduce output noise. Signed-off-by: Timofey Titovets <nefelim4ag@gmail.com>
nefelim4ag
force-pushed
the
btt-eddy-reduce-noise
branch
from
b9ff94b to
f793225
Compare
Collaborator
|
Okay, thanks. That makes sense to me. -Kevin |
Collaborator
Author
|
So, if you don't mind, then I would suggest merging this. Probably, it can make sense to add change log note that BTT Eddy may require recalibration after this. I would rebase and slightly adjust the #6813 right after that, because now I can treat F_IN > 1/4 F_REF as invalid data. Cheers, |
Collaborator
|
Okay, thanks. -Kevin |
Collaborator
Author
|
I duplicate data here, because otherwise it will be lost in the discord purgatory. Without any frequency hacks: With F_REFERECE_DIV = 2: With dummy sensor hack1 nefelim4ag@5d4b82d: Footnotes |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is draft, I want to collect data from at least 3 BTT Eddys, I think.
BTT Eddy uses 12MHz clock in frequency.
Coil is oscillating at 3+MHz.
Which is out of spec for LDC1612 sensors.
Division of coil frequency seems to reduce output noise.
So, basic idea, if we divide the coil output frequency, for some reason it improves the noise level.
This will not work for other boards, where CLKIN is adequately sized to the coil frequency.
F_REF = 12MHz.
F_REF_DIV = 1
So, basically, coil oscillating with frequency ~3.2MHz
F_COIL, F_IN_DIV = 2, makes it counts as 3.2/2 = 1.6MHz (in raw units)F_IN.So, it now satisfy the requirements of F_IN < F_REF/4.
So, how I test it, and how I suggest to test it with the current code:
So, we home the machine, we do calibration from this zero, we record the noise output of calibration routine.
How to checkout locally:
There are my outputs, did it 3 times to have an idea if it is calibration noise or divider works.
Before
After (I've forced divider = 2 for my Not BTT EDDY, there should be no significant difference):
So, for me it doesn't work, maybe even make the output a little bit noisier.
I would appreciate any data from the BTT Eddy sensors.
Thanks,
-Timofey