UltrasonicSensor hangs in some cases

[gloomyandy]

Hi,
There is no threading in the firmware. Once a native method is called it will always run until completion. So there is no need for locks or anything else within the firmware. The only time this is not true is when interrupts are involved but I don't think this is an issue in this case.

Once an i2c transaction is started it runs on a series of interrupts until completion so there is no issue in terms of gaps between the start call and the complete call.

There is something a little odd going on here. I'll put together a debug build of the firmware that will dump the internal state of the i2c code. That should help us work out what is going on...

Andy

[gloomyandy]

Hmm I may have worked out what is going on here. Can I just confirm that you are using two i2c sensors on different ports? I'll build a new version of the firmware with what I think is a fix for you to try tomorrow...

Andy

[TechnoX]

I can add that I also had this "hang up"-problem last summer.
My code was also very large and I didn't post anything on the forum because of that.
I really recognize the circumstances (with I2C-sensors on different ports and different threads etc).
When I had an integrating thread with high priority the error occured more often, but even at normal priority the sensor just hanged up after a long while.

However we didn't use the ultrasonic-sensor, but a Mindsensor, PCF8574, thermopile etc.

[unroller]

Can I just confirm that you are using two i2c sensors on different ports?

Yes, I use HT Gyro on port S1, US sensor on port S2, and HT Acceleration sensor on S4 port.

[gloomyandy]

Hi,
Please download an updated version of the leJOS firmware from here:
http://www.gloomy-place.com/lejos/i2ctest6.bin
and give it a try... I've been able to reproduce a hang when using multiple i2c sensors and with this firmware my test program runs fine... Let me know how you get on...

Note that this test build must be used with code compiled and linked with the latest leJOS snapshot. It will not work with the 0.9.0 release.
Andy

[unroller]

It seems this firmware solves the problem.

[TechnoX]

May I ask what you changed, Andy?
I have spent some days to figure this out in the C source code last summer.

I will try to rebuild my robot to be able to test this during christmas

[gloomyandy]

Hi,
the problem was with the way that the i2c_port_busy var is modified. It is typically set in the i2cStart method and cleared by the interrupt routine. The problem was that the set code was none atomic and so could end up overwriting the changes made by the interrupt code (which is atomic as it runs at interrupt time). Basically the line...
i2c_port_busy |= 1 << port;
which sets a bit in the i2c_port_busy var is not atomic with gcc on the AT91. Essentially it turns into code that is more like this...
int bitval = 1 << port;
int newval = i2c_port_busy;
newval = newval | bitval;
i2c_port_busy = newval;

The problem occurs when an interrupt happens between the initial assignment to newval and the final assignment to i2c_port_busy. If the interrupt routine makes any changes to i2c_port_busy then the change will be lost. The easy/obvious fix would be to make the above operation atomic by disabling the interrupt while the change is made (and indeed this works fine), but the disable/enable operation is actually pretty tricky with the AT91 timers...

Trying to just mask the timer interrupt did not work very well and it was easy to end up with the timer stopping. To get the AT91 timer module to generate a series of tick events you use a base clock counter and compare it to a set value, when the counter matches the compare value the event is triggered and the counter reset. The problem is that if you disable/enable the interrupt at the "wrong time" then you end up never getting any more events... I'm sure it could be made to work that way but I couldn't figure out exactly what was going on.

My second attempt simply disabled the interrupt using the advanced interrupt controller (aic) and this did indeed work. The problem is that this is considerably more complex than simply setting/clearing a bit (which is what is required to enable/disable the timer interrupt mask) and involves several method calls and disabling all interrupts for a short time. I measured the impact on the overall cpu usage and with all four ports running i2c devices this extra code added a fair bit (about 5%) to the cpu usage. I'd really rather not waste this time if possible and I don't like having to disable interrupts system wide so frequently.

So the 3rd solution which is the one in the test build was to take advantage of the way the i2c code works to retry the change made by the interrupt routine. Basically when in the RELEASE state the code checks to see if the port has been set to no longer be busy, if it has not then it clears the busy bit (but no change is made to the state), if was set to not be busy then the state is set to complete. So in effect the state will remain as RELEASE until the bit has been successfully cleared. This change handles the case of the clear being overwritten, but has a much lower impact on the cpu usage (I was unable to see any increase). It is a bit of a hack, but will be documented in the code and I think it is worth it...


Andy

[skoehler]

I had a look at the arm instruction set, hoping that I would find some instruction that are atomic and could be used to set/clear an interrupt avoiding disabling/enabling interrupts. But there don't seem to be and test and set or compare and exchange instructions available. Bummer.

Also, I have the impression that the issue stems from the fact that both the main program and the interrupt routine modify the same variable. It might have been possible to simply use a second variable like this:
i2c_port_busy1 is only modified by the main program
i2c_port_busy2 is only modified by the interrupt routine
i2c_port_busy1 XOR i2c_port_busy2 tells you which buses are busy.

To enable a bus, the main program flips a bit in i2c_port_busy1. To tell the main program that the transaction has finished, the interrupt routine flips a bit in i2c_port_busy2.

[TechnoX]

I will try to rebuild my robot to be able to test this during christmas

I have now tested my robot with the snapshot (some weeks ago) and it works like a charm!
The robot was moving around for over 1 hour, with several I2C sensors and it never got locked up. So I think the changes you made in firmware solved my problem too!

Thank you very much!