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

data is sampled on the wrong clock edgde #11

Closed
DigKleppe opened this issue Nov 13, 2021 · 9 comments
Closed

data is sampled on the wrong clock edgde #11

DigKleppe opened this issue Nov 13, 2021 · 9 comments
Assignees
Labels
bug Something isn't working

Comments

@DigKleppe
Copy link

DigKleppe commented Nov 13, 2021

The arduino shiftin function reads the data before the clock is set high. The first bit (= sign) is missed then.
The data should be read just before you set the clock low.
The delay function is just a short delayloop, mostly not needed.
I also debounce , just for sure.

// max 50us!
#define DEBOUNCES 20
bool getPinLevel ( uint8_t dataPin)
{
	int val = 0;
	for ( int debounce = DEBOUNCES ; debounce >0 ; debounce--){
		if ( digitalRead(dataPin))
			val++;
	}
	return ( val >= (DEBOUNCES/2));
}

uint8_t shiftIn(uint8_t dataPin, uint8_t clockPin, uint8_t bitOrder) {
    uint8_t value = 0;
    uint8_t i;
    delay();
    for(i = 0; i < 8; ++i) {
        digitalWrite(clockPin, HIGH);
        delay();
        if(bitOrder == LSBFIRST)
            value |= getPinLevel(dataPin) << i;
        else
            value |= getPinLevel(dataPin) << (7 - i);
        digitalWrite(clockPin, LOW);
        delay();
    }
    return value;
}

Dig

@RobTillaart RobTillaart self-assigned this Nov 13, 2021
@RobTillaart RobTillaart added the bug Something isn't working label Nov 13, 2021
@RobTillaart
Copy link
Owner

Hi @DigKleppe

Saw your remark and I will work on it as soon time permits however my agenda is quite full so it might take a while.
I updated your post to show syntax highlighting (for readability),

Thanks for posting the issue.
Rob

@RobTillaart
Copy link
Owner

Did some reading, and what I understand is that some processors are faster than others.
So they read the dataPin level so fast that the device did not have (enough) time to pull a line high,
causing effectively reading the previous bit.

This can be solved by putting an appropriate delay before the reading of the dataPin.
Effectively your debouncing also gives such delay.

Question:
Can you tell a bit about your usage, explaining when you get (expect) negative values.

@RobTillaart
Copy link
Owner

From the datasheet it the high / low time of the clock:

image

@RobTillaart
Copy link
Owner

Longer than 60 us HIGH ==> low power mode..

RobTillaart added a commit that referenced this issue Nov 14, 2021
RobTillaart added a commit that referenced this issue Nov 14, 2021
RobTillaart added a commit that referenced this issue Nov 14, 2021
@RobTillaart
Copy link
Owner

RobTillaart commented Nov 14, 2021

@DigKleppe
Created a branch - https://github.com/RobTillaart/HX711/tree/develop with a fix for shiftIn.
This fix is based upon the timing in the datasheet, and optimized for the HX711 / MS_BFIRST mode.

Can you please test it as I have no test setup nearby?

//  MSB_FIRST optimized shiftIn
//  see datasheet page 5 for timing
uint8_t HX711::_shiftIn()
{
  uint8_t value = 0;
  uint8_t mask = 0x80;
  while(mask > 0)
  {
    digitalWrite(_clockPin, HIGH);
    delayMicroseconds(1);               // T2  >= 0.2 us
    if (digitalRead(_dataPin) == HIGH)
    {
      value |= mask;
    }
    digitalWrite(_clockPin, LOW);
    delayMicroseconds(1);               // keep duty cycle ~50%
    mask >>= 1;
  }
  return value;
}

@RobTillaart
Copy link
Owner

It might be better to set the pinMode of the dataPin to INPUT_PULLUP.
That could improve the speed of rising edge.

@DigKleppe
Copy link
Author

Hi Rob, It works ok.
It runs on a 80Mhz ESP32. With the delayMicroseconds(1) the high and low times are about 3.5-4 us.
About the pullup: The datasheet does not mention that the data line is open drain. No sign of this on the scope too, nice edges. 280ns rise and 192ns fall. So pullup is not needed.
The arduino shiftin is dangerous on multiplatform use. So above solution is much better.

Note: the values you will get now are doubled compared to the previous software. get 1 bit more!
nb: i get these values out of my 1kg cell:
-18708 -18816 -18876 -18887 -18890 -18886 -18913 -18903 -18886 -18907 -18910 -18913 -18906 -18903 -18850 -18867 -18903 -18947 -18922 -18910 -18943 -18944 -18903 -18936 -19008 -18983 -18928 -18895 -18877 -18924 -18942 -18969
I see about 200 counts noise. Is this normal?

@RobTillaart
Copy link
Owner

Thanks for testing,

The doubling of the digits means that the calibration numbers will be different, I do not expect users to use the raw numbers directly. So I should mention that in the readme.md

Noise I have seen before, don't know the details, not sure but I recall smaller numbers (but that might be the factor 2 already).
It has been some time since I did work on a project with the load cells.
What I recall is that the noise depends on the loadcell used, small ones were more sensitive than large ones. Also if there is minute vibration is the setup or a 50 /60 Hz power supply can generate noise (you probably know that all too well).
The small signal of the load cells is amplified and so is the noise. Imho a noise of 200 on 19000 is ± 1% so not too bad.

How much is that noise in grams after conversion in your setup?

The library has different modi to make multiple reads and average them or take the median to "reduce" the noise.
I prefer the median mode as it is less affected by outliers and it is a real measurement.

The rise and fall times are nice, so the 1 us delay is more than enough, maybe the delayMicroseconds after the CLOCK LOW is not needed (line 299) but it keeps the duty cycle around ~50%. But for 24 bits one would gain about 24 micros, making the reading faster, but possibly more unreliable. I need to make a test setup to investigate some ideas here.

@RobTillaart
Copy link
Owner

Merged the PR into master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants