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

Suggestion: Add wrapper method to set I2C clock speed #22

Closed
regcs opened this issue Apr 24, 2021 · 23 comments
Closed

Suggestion: Add wrapper method to set I2C clock speed #22

regcs opened this issue Apr 24, 2021 · 23 comments
Assignees
Labels
enhancement New feature or request

Comments

@regcs
Copy link

regcs commented Apr 24, 2021

I am working on a time sensitive measurement, which requires a fast sampling time (< 10 ms). During my testing with the ESP32 and the ADS1115 in continuous mode according to the "continuous_4_channels" example, I found that the reads take in average 0.8 to 1 ms longer than expected.

Since this library uses the "Wire" library, I added the following function to the ADS1X15 library:

void ADS1X15::setClock(uint32_t clockSpeed)
{
  _wire->setClock(clockSpeed);
}

This enables to set the clock speed of the I2C communication. When setting the clock speed to 1 MHz, my latency went down to < 0.1 ms. Maybe, this type of function would be handy for others as well.

PS: Thanks for your nice library!

@RobTillaart
Copy link
Owner

Thanks for this idea which can be used in many other libraries as well.

Do you think there is a need for a getClock() function too? However it could not work before setting the clock as few platforms support this (directly or indirectly). Returning 0 as a sort 'don't know ' might work.

@RobTillaart
Copy link
Owner

RobTillaart commented Apr 25, 2021

Created develop branch with setClock(), getClock()

Branch also includes a new reset() which wraps the constructor initialization.

@regcs
Can you verify it works for you?

@RobTillaart RobTillaart self-assigned this Apr 25, 2021
@RobTillaart RobTillaart added the enhancement New feature or request label Apr 25, 2021
@RobTillaart
Copy link
Owner

RobTillaart commented Apr 25, 2021

Maybe the functions should be called setWireClock(), getWireClock() to make it explicit that it is not an intrinsic parameter of the AD converter.

getWireClock() should reverse the value from the lower core functions.
(for now it just returns the value set or 0 )

@RobTillaart
Copy link
Owner

getWireClock() will not make it to the lib as I cannot guarantee its correctness unless I implement a readback from the lower level I2C / TWI code for all platforms. Bit too much for now.

@RobTillaart
Copy link
Owner

@regcs
Please verify if PR works for you - #23

@regcs
Copy link
Author

regcs commented Apr 25, 2021

@RobTillaart: Thanks for taking care of that so quickly.

I just downloaded the develop-branch and tested the function. I can confirm that the setWireClock has the desired effect. But I agree that the getWireClock is probably not meaningful as long as one can not read the actual clock that might have been set by an independent function.

@RobTillaart
Copy link
Owner

@regcs
dived into it a bit and found that ESP32 supports

size_t TwoWire::getClock()
{
    return i2cGetFrequency(i2c);
}

AVR can be calculated back from TWBR register so I think I can "save" the getWireClock()

@regcs
Copy link
Author

regcs commented Apr 25, 2021

That sounds nice! I wasn't aware about the ESP32's i2c core functions - pretty handy!

@RobTillaart
Copy link
Owner

I just pushed getClock() into the develop branch.
Please verify.

@RobTillaart
Copy link
Owner

That sounds nice! I wasn't aware about the ESP32's i2c core functions - pretty handy!

Will file an issue for the Arduino core.

@RobTillaart
Copy link
Owner

Request "Please add uint32_t getClock() to the Wire library" filed at - arduino/Arduino#11457

@regcs
Copy link
Author

regcs commented Apr 26, 2021

I just pushed getClock() into the develop branch.
Please verify.

I can confirm that it works on my ESP32. Interestingly, although Espressif states a 1 MHz maximum clock speed, the maximum that I can get is 800kHz. However, this seems to be an issue of the specific hardware revision one has!? At least it was reported by others as well:

espressif/arduino-esp32#4505

This is nothing you can do anything about. Just putting it here, since it is connected to the issue and might be helpful to know for others.

@RobTillaart
Copy link
Owner

RobTillaart commented Apr 26, 2021

I've looked at I2C signals on a scope and above 400KHz one needs to tweak the pull up to get a nice (square) signal.
The reliability drops at higher speeds, but if shielded cable is used and right pull up's, good voltage, it is ok.

Note that the gain from going from 800->1000KHz is in theory 25% but in practice it is far less.
My experiments with a PCF8574 showed 400 KHz was roughly 2x as fast as 100 KHz.
If the gain == sqrt(factor frequency) assumption!! I expect that 1 MHz will at most be sqrt(1.25) ~ 10% faster in practice.

If your I2C device (e.g. EEPROM) has a write latency or so, faster busses add nothing.

BTW are you familiar with

I have no experience with them yet ...

@regcs
Copy link
Author

regcs commented Apr 26, 2021

I don't have much experience with I2C, so thanks for the explanation. If the I2C deviates from the squared signal, what is the worst that can happen? The I2C data being transferred at a wrong rate or the data being not readable at all?

I also was not aware of the spark fun endpoints, thanks! For my current project it won't be useful I guess, but it looks like it could come in handy when wanting to establish a large sensor network with a single ESP32 sitting at different places. Will keep this in mind! 👍

@RobTillaart
Copy link
Owner

If the signal is not square, bits can be read or written incorrectly. Some devices have a CRC check but most don't.

If you happen to program something expensive consequences can be quite big from a single bit mistake.
https://www.edn.com/mariner-1-destroyed-due-to-code-error-july-22-1962/
(OK not I2C but you get the idea)

A good reader about I2C - http://www.gammon.com.au/i2c - Nick has good info.

Also good are the movies of Andreas Spiess - https://www.youtube.com/channel/UCu7_D0o48KbfhpEohoP7YSQ

@regcs
Copy link
Author

regcs commented Apr 26, 2021

Haha, yeah ... that example is quite illustrative.

The videos of Andreas Spiess have been on my screen the past days a couple of times already. But thanks for pointing me to Nick Gammons side! I will definitely have a look later.

@LeboisVR
Copy link

LeboisVR commented Feb 21, 2022

Hi ! I am using an Leonardo (processor atmega 32u4). I also need to run this library as fast as possible.
How do I use "ADS1X15::setWireClock(uint32_t clockSpeed)" ? I placed
ADS.setWireClock(400000);
in my code but the arduino stay stucked...

@RobTillaart
Copy link
Owner

@LeboisVR
Better open new issues next time to keep solved issues separated from new ones.

The "Arduino stay stucked" just gives too little information to analyze the cause.

As setWireClock() is just a wrapper around the Wire library, so the cause is not expected in this library.
Maybe you can post a minimized version of your code that shows the issue.

I also need to run this library as fast as possible.

Can you explain your requirement?
What is the purpose of your project? What needs to be sampled?
How many samples per second do you need? why?
How many bits per sample do you need? why?
Which sensor do you use?

Note: I2C at 400 KHz is jus 2~2.5 x faster than 100 KHz. Most time is spend in the ADC.
Do you run in continuous mode, using interrupts, or do you poll? (continuous gives max performance)

@RobTillaart RobTillaart reopened this Feb 21, 2022
@LeboisVR
Copy link

@RobTillaart Thanks for your quick reply !
"Stay stuck": the arduino sends me millis() at each new loop, and with the line ADS.setWireClock(400000); added, it just give me the first millis().

I'll do a minimal code today or tomorrow :-)

I want to read as fast as possible. It is used for Sim racing pedals. I share my projects on my website and Discord .
So we need something like 1 read per ms.
Sensors : potentiometer or Load cells (connected to AD260 amplifier).

I do poll. I didn't tougth about interrupts. I am not sure if I can use it (many other functions in the loop). Do you have a clue about the speed gain if I do this ?

@RobTillaart
Copy link
Owner

Stuck

Other devices on the I2C bus?

I want to read as fast as possible.

That is no requirement, how many samples per second can you project process?
What is the number of bits you need?
Better question: how many states do you want to be able to detect in the pedal? 100? 255?
That tells something about the # bits you need. for 255 states a 10 bit ADC is good
(8 bits in theory but in practice you will have 1 or 2 bits of noise)
This means that a ADS1115 and even a ADS1015 is overkill (16 and 12 bits)

Do you have a clue about the speed gain if I do this ?
With continuous mode and interrupts you get the max out of the ADS (+- tolerance mentioned in datasheet)
for an 1115 this means about 800++ samples per second. If you use 4 channels that is 200++ samples per channel.

If you need a really fast 10 bit ADC, I advise you to check my MCP_ADC library as those ADC's go up to 100 K samples per second. These need to be divided over the channels. Expecting you have 3 pedals like a normal car, these MCP allows you to read all three of them in about 0.1 millisecond with a 10 bit precision (1024 levels).

@LeboisVR
Copy link

Yes there could be a mcp23017 and other things.

My project can go down to 1ms per read.
I use the joystick library that can use up to 16bits. But the industry standard seems to be 12bits, so I think I will start by moving from ADS1115 to ADS1015, but it doesn't solve the problem entirely.

Those ADC3*** seems very interesting, I will see if I can find something suitable. Thanks again !

@RobTillaart
Copy link
Owner

@LeboisVR
Shall we close the issue?
If questions arise you cab open a new one under the appropriate repo.

@LeboisVR
Copy link

Yes do it, thank you :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants