I2C Acknowledge fail STM32

[Udayan Sinha]

I have been spending some time with the STM32L100 Discovery board and have been learning to use it with the HAL libraries with CUBE. I am having some trouble in getting the I2C to work and it seems that it's not uncommon to have this problem with STM32 parts. I am trying to interface a DS1307 RTC with I2C-1 port of my STM32L100. The problem I get is that after the STM tries to talk to the RTC by transmitting the RTC device address, it does not get any acknowledge from the RTC and I end up with an acknowledge failure. Not really sure what's the problem 
My circuit and code seems just fine. I have attached my code below.
I am just using the UART, I2C and GPIOs. Other peripheral functions are not being called (even though they have been defined).

Line 96 - main()
Line 282 - I2C-1 initialization
Line 429 - UART-1 initialization
Line 467 - GPIO initialization
Line 496 - loop-based delay function

PB7-SDA
PB6-SCL

[dannyf]

I know from personal experience that the ST libraries for some STM32 parts (F1 and I think F0, maybe others) are coded incorrectly. The datasheet has the right approach in the footnote but the libraries do not follow the datasheet. ST has some extremely complicated solutions for those parts and has produced a library, with very stringent limitations, to address such an issue - it is outlined in an application note.

I do not know if those errors were carried over to the Cube. My general recommendation is to avoid ST libraries, especially the Cube.

[janoc]

Does that failure happen always or only sometimes?

I have a working code for STMF303x using the Cube/HAL libraries using essentially the stock generated code. However, I am seeing an intermittent hang when my device (an Invensense MPU-9250 IMU) doesn't ack sometimes.

If your device never worked at all, check that you are actually sending correct data and to the correct address! One common problem is that the HAL libraries expect the address to be pre-shifted one bit to the left, unlike the older STDPeriph lib code. If you don't do that, you will be sending to a wrong address and it won't work.

A scope capable of decoding I2C or a logic analyzer are going to be indispensable here.

[Udayan Sinha]

Does that failure happen always or only sometimes?

I have a working code for STMF303x using the Cube/HAL libraries using essentially the stock generated code. However, I am seeing an intermittent hang when my device (an Invensense MPU-9250 IMU) doesn't ack sometimes.

If your device never worked at all, check that you are actually sending correct data and to the correct address! One common problem is that the HAL libraries expect the address to be pre-shifted one bit to the left, unlike the older STDPeriph lib code. If you don't do that, you will be sending to a wrong address and it won't work.

A scope capable of decoding I2C or a logic analyzer are going to be indispensable here.

The failure happens everytime. I was not sure what was going on first. The voltage levels and wiring seemed fine. The code seemed okay. Then I used the in-circuit debugger and found that it doesn't get an acknowledge when it sends the device address. A sample code would be useful. Please post it here. And I don't understand. So instead of 8 bits, I would be using 9 bits for the address if I shift it to the left by one place. Is that want you mean?

I am sure that the data is correct because I have used it with PICs. And yes I don't have a logic analyzer which is kind of a problem 

[mikerj]

And I don't understand. So instead of 8 bits, I would be using 9 bits for the address if I shift it to the left by one place. Is that want you mean?

No, the address is 7 bits long but the libraries expect you to shift the 7 bit address one place to the left before calling any of the I2C functions.  The address is still a 7 bit value, but now occupies bits 7:1 instead of 6:0.

A digital storage scope is almost essential when debugging I2C problems IME.

[stmdude]

Well, schematics would have been nice, to confirm the existence of pull-up resistors, and other obvious things.

Also, the complete code would be nice.

However, in the code you attached, there's an obvious error.
You're setting the I2C addressing mode to I2C_ADDRESSINGMODE_7BIT, whilst TX/RXing to address 0xD0/0xD1, which cannot be stored in 7 bits. That's the first clue.
Also, in 7bit addressing mode, you don't (as in, should not) specify the read/write bit in the address.

The first thing I would try (stab in the dark, as I'm missing a lot of info here) is to change lines like this:
HAL_I2C_Master_Transmit(&hi2c1, 0x00D0, set_time, 9, HAL_MAX_DELAY);
to
HAL_I2C_Master_Transmit(&hi2c1, (0xD0>>1), set_time, 9, HAL_MAX_DELAY);

And
HAL_I2C_Master_Receive(&hi2c1, 0x00D1, rx_data, 7, HAL_MAX_DELAY);
to
HAL_I2C_Master_Receive(&hi2c1, (0xD0>>1), rx_data, 7, HAL_MAX_DELAY);

(Yep, same address for TX and RX. The HAL will set the RW bit depending on which function you are calling)

Also, a little warning. Take it for what it is, advice from a random guy on the Internet, but don't invest too much time in the Cube/HAL thing. It's broken in many, many subtle ways, and simply not production ready in my opinion.
Either use the old StdPeriph libraries, or bang on the hardware directly.

[Udayan Sinha]

Well, schematics would have been nice, to confirm the existence of pull-up resistors, and other obvious things.

Also, the complete code would be nice.

However, in the code you attached, there's an obvious error.
You're setting the I2C addressing mode to I2C_ADDRESSINGMODE_7BIT, whilst TX/RXing to address 0xD0/0xD1, which cannot be stored in 7 bits. That's the first clue.
Also, in 7bit addressing mode, you don't (as in, should not) specify the read/write bit in the address.

The first thing I would try (stab in the dark, as I'm missing a lot of info here) is to change lines like this:
HAL_I2C_Master_Transmit(&hi2c1, 0x00D0, set_time, 9, HAL_MAX_DELAY);
to
HAL_I2C_Master_Transmit(&hi2c1, (0xD0>>1), set_time, 9, HAL_MAX_DELAY);

And
HAL_I2C_Master_Receive(&hi2c1, 0x00D1, rx_data, 7, HAL_MAX_DELAY);
to
HAL_I2C_Master_Receive(&hi2c1, (0xD0>>1), rx_data, 7, HAL_MAX_DELAY);

(Yep, same address for TX and RX. The HAL will set the RW bit depending on which function you are calling)

Also, a little warning. Take it for what it is, advice from a random guy on the Internet, but don't invest too much time in the Cube/HAL thing. It's broken in many, many subtle ways, and simply not production ready in my opinion.
Either use the old StdPeriph libraries, or bang on the hardware directly.

Okay that makes sense. Yeah will try that. I actually took this code directly of the code I had written for a PIC over 2 years ago. In PICs, I had to account for the R/W bit too. I already mentioned the pins I am using for I2C, so I obviously had my pull-ups on those 2 pins 
But thanks, I will try out the 7-bit thingy.

I have already done almost all the other peripherals with HAL and it seems okay for me. It's a lot of code and it does take a second or so to execute the startup code, but hey I am saving time and I don't need to squeeze out every cycle right now.

[Udayan Sinha]

Still getting an error. Changed device address to 0x68 instead of 0xD0. 

[stmdude]

I have already done almost all the other peripherals with HAL and it seems okay for me. It's a lot of code and it does take a second or so to execute the startup code, but hey I am saving time and I don't need to squeeze out every cycle right now.

Yea, its not about saving time. It's just broken in a lot of ways. For instance, if you try to drive your ADCs with DMA, it will only work after a cold-boot. I.e, if you reset the system (through software, or the NRST pin), it will not work the second time around. Cold boot, and it'll work again. Once.

And, the I2C slave code is completely foobared. There's no way they even tested that, as the HAL doesn't deal with the "repeated start condition".

[Udayan Sinha]

I have already done almost all the other peripherals with HAL and it seems okay for me. It's a lot of code and it does take a second or so to execute the startup code, but hey I am saving time and I don't need to squeeze out every cycle right now.

Yea, its not about saving time. It's just broken in a lot of ways. For instance, if you try to drive your ADCs with DMA, it will only work after a cold-boot. I.e, if you reset the system (through software, or the NRST pin), it will not work the second time around. Cold boot, and it'll work again. Once.

And, the I2C slave code is completely foobared. There's no way they even tested that, as the HAL doesn't deal with the "repeated start condition".

It is about saving time. That's the whole point of having an library for the peripherals: to make it easy to use so you don't have to fiddle with the registers unless needed. Personally, all the comments about how the HAL sucks that I have seen on forums are usually 8-12 months old. ST had released HAL just about a few months before that so it is possible that it totally sucked then. But so far I have managed to get them to work and any issues that I have faced is due to mistakes on my part and not the HAL's. Haven't touched DMA yet so I can't comment on that. I2C had issues with StdPeriph libraries too so it's not just limited to HAL.

But the 7-bit thing that you suggested had no effect. I checked the I2C peripheral registers too. The address gets loaded for transmission but I get an error everytime. I can't probe the bus without a logic analyzer. 

And I don't understand. So instead of 8 bits, I would be using 9 bits for the address if I shift it to the left by one place. Is that want you mean?

No, the address is 7 bits long but the libraries expect you to shift the 7 bit address one place to the left before calling any of the I2C functions.  The address is still a 7 bit value, but now occupies bits 7:1 instead of 6:0.

A digital storage scope is almost essential when debugging I2C problems IME.

Could you please post your code so I can eyeball it? Would be really helpful.

[stmdude]

I found some old code I wrote. Lots of brokenness in it, but the I2C to the OLED-display works just fine.

At a very brief glance, it looks very similar to yours, but maybe it can help you.  I think it was for an STM32F405

BTW. Why an external RTC?   The STM has one built in...

As for the HAL, I think we have different criteria of quality. I work professionally with them, and anything less than "works 100%, in every situation, every time" is not good enough.
I do agree with you about the usefulness of libraries. All I'm saying is that the HAL is not good enough for me to ship products with. I use the StdPeriph library as much as I can, and when that fails me (which is a lot less than the HAL), I bang on the registers myself instead.
My experience with the HAL is about 4 months old.

[Udayan Sinha]

As for the HAL, I think we have different criteria of quality. I work professionally with them, and anything less than "works 100%, in every situation, every time" is not good enough.
I do agree with you about the usefulness of libraries. All I'm saying is that the HAL is not good enough for me to ship products with. I use the StdPeriph library as much as I can, and when that fails me (which is a lot less than the HAL), I bang on the registers myself instead.
My experience with the HAL is about 4 months old.

I never said that HAL is perfect. I admitted that it is inefficient as the device takes like a second or so to execute the startup code. What I said was I didn't have issues with it so far (until now maybe) and I have already done quite a bit of stuff on it so shifting to another API would mean porting all that too. Yes I am a student and a noob to the world of ARM 

And I am just using the RTC to find my way with I2C on the STM32. No other reason. It's not a "real" project. Just a learning exercise

BTW, what LCD have you used?

[AndreasF]

Only glancing at your code briefly, I noticed two issues (both of which I may have missed).

1) I don't see anywhere where you turn on the clock to the I2C peripheral (unless HAL_I2C_Init() does that).
2) I don't see any GPIO pins configured for an I2C alternative function (again, unless HAL_I2C_Init() does that for you)

[Udayan Sinha]

Only glancing at your code briefly, I noticed two issues (both of which I may have missed).

1) I don't see anywhere where you turn on the clock to the I2C peripheral (unless HAL_I2C_Init() does that).
2) I don't see any GPIO pins configured for an I2C alternative function (again, unless HAL_I2C_Init() does that for you)

Automatically handled by CUBE in another file.

[gnif]

I have found that ChibiOS contains much more reliable and well written code for the STM32 series. I used to poke all the registers myself due to the horrid libraries that ST provided and the overhead they induced, but found that the open source libstm32 was just as good, and later discovered how good ChibiOS was.

As for your issue, check that the GPIO is configured as per the reference manual for I2C, put a oscilloscope on it and make sure it is indeed transmitting. If you do not have a DSO (or CRO even), slow down the I2C bus and put some LEDs on it, hopefully you will see some life. Honestly the best investment you can make when working with these devices is a DSO, the Rigol models even support I2C decode which is very handy when you have strange issues like this.

[Udayan Sinha]

I have found that ChibiOS contains much more reliable and well written code for the STM32 series. I used to poke all the registers myself due to the horrid libraries that ST provided and the overhead they induced, but found that the open source libstm32 was just as good, and later discovered how good ChibiOS was.

As for your issue, check that the GPIO is configured as per the reference manual for I2C, put a oscilloscope on it and make sure it is indeed transmitting. If you do not have a DSO (or CRO even), slow down the I2C bus and put some LEDs on it, hopefully you will see some life. Honestly the best investment you can make when working with these devices is a DSO, the Rigol models even support I2C decode which is very handy when you have strange issues like this.

Was considering a Analog Discovery because it has a logic analyzer+oscilloscope+signal generator all-in-one (when I save enough money). Plus I get a student discount too.

Will try out the LED thing though.

[AndreasF]

...
Automatically handled by CUBE in another file.

OK.

Having a quick look at the DS1307 datasheet, I agree with gnif. I would probably drop the I2C clock speed down a bit, since you're currently right at its maximum value.

[Udayan Sinha]

...
Automatically handled by CUBE in another file.

OK.

Having a quick look at the DS1307 datasheet, I agree with gnif. I would probably drop the I2C clock speed down a bit, since you're currently right at its maximum value.

No I am not 
Far from it, I am running it a 100kbps. Can't go any slower now

[gnif]

Will try out the LED thing though.

Just a futher thought, if you try the LED method out, you may need to buffer it so as to not screw with any communications on the line. I would use a 74LS17 or 74LS16 (inverting), to provide the drive current for the LED.

You could also slow down the I2C clock further by underclocking the APB with a higher divisor.

[AndreasF]

No I am not 
Far from it, I am running it a 100kbps. Can't go any slower now

No??? I'm not sure what makes you think that.

[janoc]

No I am not 
Far from it, I am running it a 100kbps. Can't go any slower now

No??? I'm not sure what makes you think that.

@Udayan, Apropos, are you sure you are actually running at those 100kbps? Cube did misconfigure the system clock for me a few times. You really need a scope here and check what it being output on those pins (if anything). Otherwise you are shooting completely in the dark - it could be even as silly issue as a missed pull-up resistor or poor contact on it.  And you will be wasting your time fiddling with code which has nothing to do with the issue.

[Udayan Sinha]

No I am not 
Far from it, I am running it a 100kbps. Can't go any slower now

No??? I'm not sure what makes you think that.

@Udayan, Apropos, are you sure you are actually running at those 100kbps? Cube did misconfigure the system clock for me a few times. You really need a scope here and check what it being output on those pins (if anything). Otherwise you are shooting completely in the dark - it could be even as silly issue as a missed pull-up resistor or poor contact on it.  And you will be wasting your time fiddling with code which has nothing to do with the issue.

Okay I did the following stuff:
1) I rechecked all connections and voltage levels and they all seem fine.
2) I tried it with another DS1307 that I had. No improvement.
3) I tried to define the GPIO functions manually for the I2C pins (they were already defined in another file). No improvement again.
4) I stuck an LED on the bus. There is a very slight flicker in the LED brightness when the communication begins but afterwards it just stays. I reduced the bus speed to 1bps as well and the result was the same. So it seems that the STM32 is not driving the bus 

[AndreasF]

The flicker at the beginning would suggest that something is happening, at least initially. I would put another LED on an unused pin and then toggle it in your main while(1) loop. That way you at least know if the while loop keeps going or if the program got stuck somewhere (I suppose your UART output should already tell you this).

One thing you could try (since you don't yet have an oscilloscope or logic analyser) is to use the audio line in of your computer as a quick "quasi" oscilloscope (aka "sound card oscilloscope"). It should work up to a few tens on kHz (don't go too low - they are typically AC coupled). That way you should be able to at least verify that the clock speed is as you expect it to be.

Edit:
It might also be worth it to keep track of the HAL_Status that is being returned from the different function calls. At least it'll tell you if there are errors, timeouts etc.

[Udayan Sinha]

The flicker at the beginning would suggest that something is happening, at least initially. I would put another LED on an unused pin and then toggle it in your main while(1) loop. That way you at least know if the while loop keeps going or if the program got stuck somewhere (I suppose your UART output should already tell you this).

One thing you could try (since you don't yet have an oscilloscope or logic analyser) is to use the audio line in of your computer as a quick "quasi" oscilloscope (aka "sound card oscilloscope"). It should work up to a few tens on kHz (don't go too low - they are typically AC coupled). That way you should be able to at least verify that the clock speed is as you expect it to be.

Edit:
It might also be worth it to keep track of the HAL_Status that is being returned from the different function calls. At least it'll tell you if there are errors, timeouts etc.

Yes I planned to do all of that after last night. I already know the loop is running because of the UART output (like you guessed). And the flicker occurs on both lines (clock and data) which is weird. I think I will go to the university lab tomorrow with the circuit and probe it with a DSO there.

I already know that it's returning errors (I used the in-circuit debugger through Keil). And timeout is not possible as I have used HAL_MAX_DELAY as the timeout value (which is like 50 days) 

[richardman]

I hate to be so frank, but why people spend so much time fighting bugs is beyond me. Eat ramen for a month and get a USB logic analyzer, then you know for sure what's happening with the signal.

Spend a couple days port to a real API (I am partial to our JumpStart API, as I have debugged it to work with 8/16 bits I2C on varying speed on both the F0xx and F4xx, but I am sure ChibiOS and others are just as code) and get the code working so you can do the actual useful stuff. All you are learning now is how badly an API can be implemented or that you are missing a crucial bits of initialization because either the doc wasn't clear or you just miss it. This is not learning embedded system programming, this is learning to bash your head against the wall. 

[janoc]

@Richardman - I would normally agree with porting to a saner API, but that's not always an option. Also ChibiOS doesn't expose all the functionality you may need - their HAL and drivers expose only the basics, anything more platform specific and you are back to writing into registers.

One approach that has worked for me when trying to make some STM32 peripherals work was to examine the peripheral registers and compare with the reference manual whether the values are sensible. Basically walk through the documentation for the peripheral from top to bottom and check each register. STM32 has most registers working in such way that the defaults (zeros) are typically the values you will most commonly want, so not many bits need to be set. That approach let me discover some bits that the STDPeriph library wasn't setting and thus my code wasn't working. The same with ChibiOS or Cube/HAL.

Tie this with some hardware analyzer - either a scope or logic analyzer and you are golden. I prefer scope for the initial troubleshooting, because a logic analyzer will not show issues like a signal distortion due to two chips attempting to drive the line at the same time (e.g. because pin mode has been configured wrong or assigned to an incorrect alternate function).

The HAL is bloated, poorly documented and buggy, but it is not THAT unusable. It works for many common cases without issues. Whether or not you use HAL makes no difference if you are debugging without adequate instruments - you will be still facing the same issues trying to make ChibiOS or whatever else to work.

[richardman]

Definitely agree that the OP should get a scope or analyzer. A USB based one is around $100. That would save  days of work

[kannan]

i am issueing the same problem can you please give me your code

[janoc]

i am issueing the same problem can you please give me your code

Dude, you realize that this thread is 8 years old, right?