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

Removed second delay on eeprom writes #2

Closed
wants to merge 3 commits into from

Conversation

David-OConnor
Copy link

This PR removes the second delay during EEPROM writes. This delay is due to the note in the datasheet that a delay of 5ms (conservatively 10ms) between eeprom writes is required to prevent bugs. I've confirmed this: Wacky stuff happens if you don't delay!

This PR is due to the delay being inconsistent: We need the one between the writes. Currently, the onus is on the user to execute delays otherwise, ie between reads, or between read and write commands. I see two options:

#1: This PR as-is, ie removing all internal delays except the one between two writes. The user must insert delays etc in their code.
#2: Add a delay after or before each read. This would avoid the user having to add delays, but adds latency in situations where it may not be needed

@eldruin
Copy link
Owner

eldruin commented Jan 22, 2021

The delay is necessary after EEPROM writes, not before or after EEPROM or RAM reads. So I think the current state is correct.
Did I miss something else?

@David-OConnor
Copy link
Author

David-OConnor commented Jan 22, 2021

Anecdotally, if I don't insert manual delays between reads, or reads and writes, the device behaves unpredictably, eg crashes, setting the wrong value etc. So, I've been treating it as required between all operations. Could this be because the read process involves writing the address?

For example, this works:

    let mut temp = sensor.object1_temperature().unwrap();
    delay.delay_ms(10);
    let emis = sensor.emissivity().unwrap();

But this doesn't:

    let mut temp = sensor.object1_temperature().unwrap();
    let emis = sensor.emissivity().unwrap();

I chose this approach in the PR instead of adding delays before or after each read, since perhaps you won't want the delay if you're using a wfi between readings anyway.

@eldruin
Copy link
Owner

eldruin commented May 22, 2021

I have removed the delay as suggested here, thanks!
I have also implemented support for device sleep/wake and released everything in version 0.2.0.
Closing.

@eldruin eldruin closed this May 22, 2021
@David-OConnor
Copy link
Author

Sweet

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