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

Issue2524 allow clocks below 100KHz #6934

Merged
merged 11 commits into from
Dec 28, 2019
Merged

Issue2524 allow clocks below 100KHz #6934

merged 11 commits into from
Dec 28, 2019

Conversation

Tech-TX
Copy link
Contributor

@Tech-TX Tech-TX commented Dec 22, 2019

Corrects the fixed I2C bus clock selections and allows slower bus clocks down to ridiculously low bus speeds if you need that. Originally prompted by Earle's comment in #2524 about computing the bus clock timing.

Additionally, the dead Twi::stop was removed. The compiler was removing it in slave compiles anyways, as it wasn't called by any of the functions except for one oversight in the slave state machine. Slaves can't issue STOP commands. Although the Twi::stop was listed as an exposed function, it wasn't linked from Wire.h, so it was optimized out.

@Tech-TX
Copy link
Contributor Author

Tech-TX commented Dec 22, 2019

BTW, here's a variation on the Master_Writer example so you can knob the bus clock from the serial terminal. Enter your desired speed followed by enter, and it changes the Wire.setClock(freq) immediately. It also displays the half-cycle period and how many loops busywait() will execute.

#include <Wire.h>
#include <ESP8266WiFi.h>
#include <PolledTimeout.h>
#define SDA_PIN 4
#define SCL_PIN 5
const int16_t I2C_MASTER = 0x42;
const int16_t I2C_SLAVE = 0x08;
String i2c_clock = "";   // stores the frequency request

void setup() {
  Serial.begin(74880);           // start serial for output
  Serial.println();
  Serial.println("Starting...");
  Wire.begin(SDA_PIN, SCL_PIN, I2C_MASTER); // join i2c bus (address optional for master)
  Wire.setClock(100000L);
  WiFi.persistent(false);
  WiFi.mode(WIFI_OFF);
  delay(100);
  WiFi.forceSleepBegin();
  yield();
}

byte x = 0;

void loop() {
  using periodic = esp8266::polledTimeout::periodicMs;
  static periodic nextPing(100);

  if (nextPing) {
    Wire.beginTransmission(I2C_SLAVE); // transmit to device #8
    Wire.write("x is ");        // sends five bytes
    Wire.write(x);              // sends one byte
    Wire.endTransmission();    // stop transmitting

//    Serial.println (x);

    x++;
  }
    if (Serial.available()) {
    i2c_clock = Serial.readStringUntil('\n');  // enter the I2C bus speed in the serial monitor
    Serial.print("bus clock = ");
    Serial.println(i2c_clock);
    unsigned int freq = (i2c_clock.toInt());
    unsigned int period_nS = (500000000/freq);  // half-cycle period in ns
    Serial.print("period = ");
    Serial.print(period_nS);
    Serial.println("ns");
#if F_CPU == FCPU80
    unsigned int twi_dcount = (1000*(period_nS - 1120)) / 62500;  // (half cycle period - overhead) / busywait loop time 
#else
    unsigned int twi_dcount = (1000*(period_nS - 560)) / 31250;  // (half cycle period - overhead) / busywait loop time 
#endif
    Serial.print("busywait = ");
    Serial.print(twi_dcount);
    Serial.println(" loops");
    Serial.println();
    Wire.setClock(freq);
    }
}

Copy link
Contributor Author

@Tech-TX Tech-TX left a comment

Choose a reason for hiding this comment

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

@devyte - [review integer ranges for overflow, minor constness ]

edit: divide by zero if they feed it 0Hz, uint32_t overflow below 233Hz
edit2: there's no overflow, my test program above was mixing ints and unsigned ints (corrected). The real code was correct. I knew I'd checked my math twice to insure I stayed within an unsigned int!

Might want to limit the lower end even further, as a single byte read or write is ~78ms at 250Hz.
(start, 7 bits address, read/write, ack, 8 bits data, ack, stop). Maybe somewhere around 500Hz to 1KHz would be a better lower limit. I don't expect any more than a few people trying to run it lower than 10KHz, normally. There were some radio PLLs that needed ~ 10KHz bus clock back in the '80s, but that's ancient history.

@Tech-TX Tech-TX changed the title Issue2524 Issue2524 allow clocks below 100KHz Dec 22, 2019
@devyte
Copy link
Collaborator

devyte commented Dec 27, 2019

Might want to limit the lower end even further

I think a 1KHz lower limit is ok, I somehow can't picture I2C at such a low clock speed lol.

Copy link
Collaborator

@devyte devyte left a comment

Choose a reason for hiding this comment

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

Nice work here. Only minor changes before merge.

cores/esp8266/core_esp8266_si2c.cpp Outdated Show resolved Hide resolved
cores/esp8266/core_esp8266_si2c.cpp Show resolved Hide resolved
cores/esp8266/core_esp8266_si2c.cpp Show resolved Hide resolved
@devyte devyte merged commit 3197d2a into esp8266:master Dec 28, 2019
@devyte devyte added this to the 2.7.0 milestone Dec 28, 2019
@devyte devyte mentioned this pull request Dec 28, 2019
@Tech-TX Tech-TX deleted the issue2524 branch December 28, 2019 23:21
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.

None yet

2 participants