Software I2C

[fm-33]

Hi, folks! First of all, thanks for providing this fantastic library!

I would like to know if you can add an overload method begin(TwoWire) to support software I2C connections. I think this would solve many compatibility issues reported by users of those small boards.

(Please forgive me if this is not the right place to ask this. I'm an old programmer but a newbie using Git)

[edgar-bonet]

Somewhat related to PR #160.

Sounds like a good idea, but I doubt there is the manpower here to implement it. You may try to do it yourself and submit a pull request.

[fm-33]

Gonna to try it! I have just DS3231s and another module (which I don't recall its model and have no idea of its whereabouts). Can I submit the code with just DS3231 begin(TwoWire) overload?

[drak7]

It'd be good to have for all of them, if it's working for the DS3231 then it should work for the others. I (or other intrepid users) can test it on the other modules.

[fm-33]

I think I did it (at least looks like it is working, LOL). I probably not followed the best practices because of the hurry, but I tested with other devices using the same I2C bus (3 ADS1115) and, every scenario was "perfect" here. My scenarios: Hardware: ESP32 TwoWire clock: 100 kHz

1. Alone using the I2C hardware pins;
2. Together with one ADS1115 using the I2C hardware pins;
3. Together with three ADS1115 using the I2C hardware pins;
4. Using different GPIOs (used at least six different pins).

How can I give you the code? (Please remember that I am a "Git-newbie", LOL) PS. As I feared many errors, my custom library has just the DS3231 class/methods and the common methods. PS II. I used Adafruit_ADS1015.h library.

[edgar-bonet]

How can I give you the code? (Please remember that I am a "Git-newbie", LOL)

If you have never used git before, beware that there is a significant learning curve. Somewhat intimidating at first but, once you know it, an extremely useful tool for collaborative coding. You may want to start by watching a tutorial or two. Then, you have to install it. On Linux, you can use the version provided by your package manager (e.g. sudo apt install git). Otherwise go to the download page and pick the one that fits your platform.

While you are at it, you will also need clang-format for ensuring conformance with this project's formatting conventions. On Linux, sudo apt install clang-format, or equivalent for other package managers. If you are using another OS, I don't know how to get it. I guess it is part of the “various LLVM tools” shipped within the clang pre-built binary packages.

First thing once you have git is to tell it who you are. This is your identity as will be seen by people looking at the history of the project. On a terminal (“git bash” on Windows):

git config --global user.name 'Your Full Name'
git config --global user.email 'your.email@your.domain'

Now, create a couple of clones of the RTClib repo: one on GitHub (this one is called a “fork”) and one on your local machine. On GitHub, click on the “Fork” button near the top right of the RTClib repo page. On your computer, I assume you already have a directory named RTClib with your changes. We will rename it “modified-RTClib” and create a new RTClib directory which will be your local git repo. The directory structure should end up like this:

common parent directory
├── modified-RTClib  # the version you have modified
└── RTClib           # your local git repo and working directory

For this, move to the parent directory and type:

mv RTClib modified-RTClib
git clone https://github.com/fm-33/RTClib.git
cd RTClib

Now you can create a new branch here and get your changes ready for committing:

git checkout -b soft-i2c
cp -a ../modified-RTClib/RTClib.{h,cpp} .
clang-format -i RTClib.{h,cpp}
git diff

The clang-format command may slightly reformat your code. The last command will let you review the changes. Make sure there are no changes that are not relevant. If all looks good:

git commit .

This will drop you in an editor for writing a log message. Write a short title, an empty line, and some descriptive message, ending in 'Fixes #172.'

Time to push this new branch to your fork:

git push --set-upstream origin soft-i2c

GitHub will now ask whether you want to create a pull request. Accept, and your first pull request will show up on this repo.

[fm-33]

Thank you very much for your response, Edgar! It will surely help me to learn how to use Git. While I will learn how to use Git, I think I will let the code at Arduino Forum for other people test it too. Is it ok?

[edgar-bonet]

@fm-33: I personally wouldn't expect anyone to object to that, but note that I am just a random small contributor to this repo.

[fm-33]

@edgar-bonet Finally had the time to post the code at Arduino forum for other users test it. https://forum.arduino.cc/index.php?topic=692007 If you can test it, give it a try!

[fm-33]

@edgar-bonet @drak7 Hey, guys. Have you tested it? I am using the custom library just with my DS3231, and no problem till now. I am still stuck with my "Git classes", that's why I didn't request a push/commit (or something like that).

[edgar-bonet]

I cannot test it, for lack of appropriate hardware.

I just took a look though, so here is some feedback:

• There seem to be large amounts of whitespace changes, including the end-of-line convention changed from LF to CRLF. Although inconsequential for the functioning of the code, this is something to be avoided on any collaborative project. It makes the diff big and noisy, the code review more difficult, and reduces the usefulness of git blame. Note that the CI pipeline now requires you to run the sources through clang-format, so those changes have to be undone anyway for the automated tests to pass.
• If I understood correctly, the variable soft_i2c_connection can point to either the Wire object (hardware I2C), or a soft implementation. If this is the case, then the name is badly chosen. A more generic name, like i2c, would be more appropriate.
• The structured Doxygen comments are meant to be automatically included in the official reference of the library. The comment “Added for the test” is inappropriate in this context.
• If the user forgets to call begin(TwoWire *), the pointer soft_i2c_connection will not be initialized, and any call to a method that uses it will result in a crash. It would be preferable for the pointer to be initialized by the constructors. Better yet, use a reference rather than a pointer if possible. See below.
• If the user calls the no-parameter version of begin(), the pointer is not initialized either. This means that all sketches using the current API will crash if this change is merged as is.
• This seems to be based on RTClib 1.8.0. There have been some changes to RTClib since then, so it would be a good idea to rebase on top of master before submitting a pull request.

As a solution to the problem of the pointer to the I2C implementation being possibly uninitialized, I suggest having all the hardware-RTC classes follow this pattern:

class RTC_XXX {
public:
  RTC_XXX(TwoWire &i2c_port = Wire) : i2c(i2c_port) {}
  boolean begin(void);
  // other methods...
private:
  TwoWire &i2c;
};

The user would then have the choice to instantiate the RTC as

RTC_XXX rtc(my_i2c_implementation);

or just as usual

RTC_XXX rtc;

in which case Wire would be used, preserving backwards compatibility.

[fm-33]

Thanks, Edgar! I know that the code is a mess LOL, I wrote it in 2 hours. This version is just for tests. When I learn all this Git stuff, I'll surely write it as you are telling me to!

[ladyada]

are you sure you want software i2c support and not just being able to specify the i2c pins for Wire?

[benhadad]

are you sure you want software i2c support and not just being able to specify the i2c pins for Wire?

I am wondering what the performance penalty is?

@edgar-bonet thanks for your mini Git Class above, Even though I have been using Git since 2005ish it still nice to users so politely helping other users.

[drak7]

Implemented in #231