Wish there were hardware based Wire Library

[KurtE]

I wish that the Wire library was using the underlying hardware I2C instead of the bitbanging I2C code.

Also wish that it exported a Wire1 object which uses the I2C2 hardware object which currently defaults to Arduino pins 50 and 51.

I have started working on it and have a partial version converted, that can compile for either software I2C output or hardware. There are several things I that I have not tried to do yet, like support slave mode, not sure about how to implement the sendStop parameter of endTransmission as the underlying HAL calls don't know anything about this.

But: if you want to take a look at the WIP, it is up in new branch (based off of my SPI enhancements branch)

https://github.com/KurtE/OpenCR/tree/Wire-Library

I have included here an updated version of the I2CScanner program that came from Arduino playground. I hacked up the version that ships with Teensyduino, to allow you you to configure it for which Wire object... I2CScanner-180622a.zip

I have also done some testing using one of my own test programs I have been trying out adding three VL6180 sensors from Adafruit using with of Adafruits I2C Expanders. And again updated it to allow me to choose which wire library to use. Test_3_vl6180x-180622a.zip

Other things I have not tried/tested is to set the buss speed. I have code in place that can set one of 3 frequency values. I am using the magic numbers for the i2chandle.Init.Timing values. Not sure how many we should try adding: maybe 100000, 400000, 1000000 any others.

I will try to continue to flesh out a few more things, unless someone beats me to it ;)

[OpusK]

Cool! HW support for the I2C library was on our roadmap, but was delayed due to other tasks...

Our first plan was as follows.

• SW/HW selection via class constructor parameter. (Through the pin number... like below code)

  
  TwoWire::TwoWire()
  {
  hw_mode = true;
  
  setFrequency(400);
  
  ...
  }

TwoWire::TwoWire(uint8_t sda, uint8_t scl) { hw_mode = false;

si2c = new SlowSoftI2CMaster(sda, scl);

... }

However, the HAL library is not suitable for the Wire library method.
So, for some functions, I think it need to approach the registers directly. (like beginTransmission() etc ..)

[KurtE]

Thanks @OpusK ,

I started off playing with it as handling both hardware and software I2C, like you mentioned. On the Hardware side, my constructor,

TwoWire::TwoWire(I2C_TypeDef *i2c_instance) {
  i2c_handle_.Instance = i2c_instance;
}

I didn't put in any of the setFrequency stuff here as I did not want it to do anything hardware wise in the constructor as not knowing state of board when C++ constructors are called... Put the call to setClock into the begin method instead.

With the above I have:

TwoWire Wire(I2C1);
TwoWire Wire1(I2C2);

My thinking was if we have real hardware I2c working would we need/want software version as well? So I currently (in the github link mentioned above), I have compile option to use one or the other.

Another option was to create TwoWire as a class with more virtual functions, and create two sub classes (software, hardware), where some of the main functions are handled by the subclasses.

With my first pass, I needed to modify the HAL library, to allow HAL_I2C_Master_Transmit to function if I pass in a count of 0. That is someone calls Wire.beingTransmission(0xe0); followed by Wire.endTransmission(); without any calls to write(). I know that some programs use this to verify if an I2C address is valid. Example the I2CScanner I uploaded in first comment.

As you mentioned the current HAL code does not handle all of the capabilities. So the question would be, to not use the HAL code or to try to extend it. Will still play some to see how hard it is to get more of it to work.

[OpusK]

I needed to modify the HAL library, to allow HAL_I2C_Master_Transmit to function if I pass in a count of 0.

In my opinion, it is better not to modify the HAL library. I do not know if we are going to update the HAL library to the latest version, but it looks like it should go directly to setting the registers, if necessary, without modifying the external library.

That is someone calls Wire.beingTransmission(0xe0); followed by Wire.endTransmission(); without any calls to write().

Some of the libraries I know are doing I2C functions using only beginTransmission() and endTransmission(). Therefore, going to the same API as TwoWire seems to be good for compatibility.

I didn't put in any of the setFrequency stuff here as I did not want it to do anything hardware wise in the constructor as not knowing state of board when C++ constructors are called... Put the call to setClock into the begin method instead.

I totally agree with you. If the user has to set the speed through begin (), it seems to be necessary to create an API that gives the parameter the default value (100Khz?). When the user types it, it changes accordingly. And I think it would be enough to support only 100K, 400K, and 1M.

[KurtE]

Given, the idea of not modifying the official HAL library, I am assuming that I will simply copy out the functions that need modifications and then make the needed changes.

I don't know if an Alternative would be try to get the STM people take some Pull requests to HAL? The change I currently made is not difficult, but simply remove the test for 0 bytes, and change the: loop from do {} while (count); to while (count) {};

But looks like the changes might be a little more involved with handling the send stop and restart. Looks like these are handled by the I2C_CR2 register? If not send stop (don't set I2C_CR2_AUTOEND). Then the restart should happen if you call off to do the set the start bit (I2C_CR2_START)... I will try to play some over the next day or two of making copy of those functions into my Wire code and play with setting or not setting those bits. Hopefully we can still use most of the HAL helper functions. Hopefully things like I2C_TransferConfig can be used. Will have to verify by making sure the asserts will be valid

[KurtE]

Quick update -

I am working on version with hopefully with SendStop and restart support. I think most of it is in place for Transmit, but maybe not receive. I think I need to add in some of the same stuff I have on the Transmit side. That is how it detects ending changes depending on the options.

I am also starting to put in support for I2C Client. I am trying sort of a simple test case where I have SPI1(Wire) connected up to SPI2(Wire1). I have Wire.begin() set of master, and Wire1.begin(CLIENT_ID) as slave. and so far the test program takes text from Serial and then transmits it on Wire... So far the Wire1 code has the starts of the Receive. However I think the Slave code is hanging the SPI. That is I know that Wire1 thinks it has matched address... But I don't believe the ISR is being called. I am trying to have the client begin code setup to call HAL_I2C_Slave_Receive_IT, After my init code finishes, I have it dump some information about the two I2C objects:

Start Wire to Wire1 test
I2C1 ISR:1 CR1:1 CR2:2000000 OAR1:0
I2C2 ISR:1 CR1:fd CR2:2000000 OAR1:80cc

So I believe it is init to enable interrupts on most everything except transmit(cr1).

So I then try to output some text and:

Enter Text to send: abcd
I2C1 ISR:8001 CR1:1 CR2:20420cc OAR1:0
I2C2 ISR:cc8009 CR1:fd CR2:2000000 OAR1:80cc

So it looks like it shows an address match ISR with bit 0x8...

But I don't think the ISR I think that is supposed to be called: void HAL_I2C_EV_IRQHandler(I2C_HandleTypeDef *hi2c) Is not being called (I believe), as I added a call: HAL_GPIO_WritePin(GPIOB, GPIO_PIN_14, 1); // digitalWrite(12, high); to the start of it and likewise call and end with value 0... And I am not seeing anything show up in the Logic analyzer for IO pin 12... I did a pinMode(12, OUTPUT) in the setup function in sketch.

Still debugging. But if anyone wants to take a look at the WIP, I pushed up the current stuff to same branch: https://github.com/KurtE/OpenCR/tree/Wire-Library Currently still based off of my SPI stuff. Will rebase with develop once this is working mroe.

can also include simple test sketch I am trying if anyone wishes to look

Edit: Forgot to mention, that current code in test, When the Wire1 code looks like it should call the ISR. The SPI buss is left with the clock stretched out...

[KurtE]

Progress...

Turns out there is no GLUE setting up the ISRs for I2C. Took me awhile to verify and figured this out two different ways.

Used: Objdump and looked at vector table plus none of the code associated with the I2C ISR was in the executable... Used command:

C:\Users\kurte\AppData\Local\Temp\arduino_build_873997>C:\Users\kurte\AppData\Local\Arduino15\packages\OpenCR\tools\opencr_gcc\5.4.0-2016q2\bin\arm-none-eabi-objdump -D Wire_Master_To_Slave_test.ino.elf > foo

I then the was doing a search on who was supposed to setup the ISR linkage and found nothing, so then looked at Uarts and found their linkage was done in drv_uart.c with functions like:

void USART6_IRQHandler(void)
{
  HAL_UART_IRQHandler(&huart[DRV_UART_NUM_1]);
}

So did the same sort of thing in drv_i2c.c, this updated the vector table, but still my code was not called. Then figured I needed to add:

   /* Peripheral interrupt init */
    HAL_NVIC_SetPriority(I2C1_EV_IRQn, 0, 0);
    HAL_NVIC_EnableIRQ  (I2C1_EV_IRQn);
    HAL_NVIC_SetPriority(I2C1_ER_IRQn, 0, 0);
    HAL_NVIC_EnableIRQ  (I2C1_ER_IRQn);

Likewise for I2C2... in the function: HAL_I2C_MspInit

So now my interrupt handler is getting for the first buffer received... Now to debug and extend. Example I now know I need to again call of to the HAL_I2C_Slave_Receive_IT After I process the resulting call...

More still to figure out, like how to setup for both transmit and receive... But at least I am starting to see some progress...

[OpusK]

@KurtE,

When I look at your commit, it seems that there is some solution to the ISR related part, right?

I have responded to robotsource for information related to vector tables.

[KurtE]

@OpusK,

Thanks for the link to the programming manual with the ISR stuff. Still think that maybe should document and also maybe take look through to decide if current ISR priority levels make sense and if apps would want to be able to tweak these.

I have made progress on getting the ISR to work. Lots more details up on the RobotSource thread: https://community.robotsource.org/t/wire-library-on-opencr/1444

I also create a different branch of OpenCR that was based on current develop branch: https://github.com/KurtE/OpenCR/tree/Wire_develop

I have been tempted to create a Pull Request as the Master code is working better. However I have not tested the sendStop/restart code parts as have not looked for a test case that uses it.

Things that are interesting to try out and maybe tweak: a) Clock speeds, I set up the library code to handle trying to set for 100K, 400K, 1000K speeds. For the first two I use the values from other library and from @ROBOTIS-Will camera example. Not sure where/how these were derived. The both produce values >100K and > 400K. For the 1000K I used the values out of reference manual for example I2C clock speed 48mhz, Which appears to be running slower than 1000K probably I am guessing I2C clock speed is probably 54mhz (216mhz/4)?

b) Some test code that uses Restart.

The Slave code appears to be working fine for receiving data from Master. Still some issues on the slave sending data back. Sometimes it does not send all of the bytes back (misses last one), and then picks up that last byte in the next time the master asks for data... Again test app, has Wire connected to Wire1. Wire_Master_To_Slave_test-180702a.zip

Also in the code I needed to create another HAL slave ISR mode. That is the HAL code had calls to say setup for a Slave Receive or it had a call to setup for a Slave Transmit, but the Wire library and I believe I2C in general is supposed to be:
Slave waits for an I2C transaction where i't slave ID is matched, it then looks at the direction bit to then setup for either a transmit or a receive.

Again lots more details on current run of test app on the Robot source thread.

[OpusK]

100K, 400K, 1000K speeds. For the first two I use the values from other library and from @ROBOTIS-Will camera example. Not sure where/how these were derived.

Please refer to Section 30.4.9 of the Reference Manual for i2c timing. These values were used to refer to the values in the STM32 CubeMX example.

[KurtE]

Thanks, I was looking at the same tables. As I mentioned in the above comment, for 1000K, my first guess was from manual... That ws the section you mentioned(30.4.9). As I mentioned, I tried the examples from the 48K table, (5, 3, 1, 0, 1), but the values did not appear to work correctly. Again I am assuming that the fI2CClock is 54mhz (216/4).

I tried to partially configure up the STMCubeMX program. I only configured up a few things like, I have I2c1 and I2c2 and played some with the clock settings to get it to 216mhz for system clock and 54mhz for i2c1 and i2c2...

I go to the configuration page, double click on I2c1 and it brings up configuration stuff including the calculated timing values. If I go by the default for 100K I see: 0x20404768 Which is completely different than the values I am using from other library... : 0x40912732

But maybe you have some other things set, like; rise time, fall time, Defaults to analog filter and 0 for digital filter.

Is the STM32 CubeMX project you use, available? I did not find any *.ioc files?
Or if you use your setup, what values do you get for 1000K?

Thanks

[OpusK]

What we refer to were some examples that already existed. These exist in the following locations. If there is a repository, I'd like to give you a link, but I can not find it.

STM32Cube_FW_F7/Projects/STM32746G-Discovery/Examples/I2C

For 1M, there is a case of "SYSCLK = 72Mhz" in the example, but we have to calculate separately for the 216Mhz we use. I do not use the CubeMX program, so I do not know how the program will calculate it.

In this regard, I will try to calculate it myself.

[OpusK]

@KurtE, What you said is right.

The timing we used earlier was 50Mhz for the I2C clock, and 54Mhz for now, so the timing you mentioned is correct.

• 100K : 0x20404768
• 400K : 0x6000030D
• 1M : 0x00200922

In this regard, we will fix it.

[KurtE]

Thanks: Timings look better 100K: logic analyzer now shows at 98K 400K: ... 369.3K 1M: ... 830.6K

Probably can close this issue...