I2CDevice throws System.InvalidOperationException after sdk update

Hello

Board: G120

After I updated to 4.3.8.1, this code started throwing a System.InvalidOperationException

 I2CDevice.Configuration config = new I2CDevice.Configuration(ADDRESS, 250);
I2CDevice idDevice = new I2CDevice(config);

The stacktrace shows that it’s thrown from Microsoft.SPOT.Hardware.Port::ReservePin
which suggests it might be a conflict but this is the only I2CDevice that is beeing used.

The code works with 4.3.6.0 and 4.3.7.10.

Any ideas?

Turned out the code was actually called from 2 places in the code, so it could be a race issue, but I don’t understand why updating the sdk would trigger that.

I thought i’d try adding a lock but got a behaviour I didn’t expect. This is the original class:


using System;
using Microsoft.SPOT;
using Microsoft.SPOT.Hardware;

namespace Gateway.Utils
{
    public static class EPROMReader
    {
        private const byte ADDRESS = 0xA0 >> 1;
        private const byte MAC_ADDRESS_REG = 0xFA;

        public static byte[] ReadMAC()
        {
            byte[] id = new byte[8];

            I2CDevice.Configuration config = new I2CDevice.Configuration(ADDRESS, 250);
            I2CDevice idDevice = new I2CDevice(config);
            I2CDevice.I2CTransaction[] i2cActions = new I2CDevice.I2CTransaction[2];

            byte[] registerNum = new byte[] { MAC_ADDRESS_REG };
            i2cActions[0] = I2CDevice.CreateWriteTransaction(registerNum);

            byte[] registerValue = new byte[6];
            i2cActions[1] = I2CDevice.CreateReadTransaction(registerValue);

            idDevice.Execute(i2cActions, 50);

            id[0] = registerValue[0];
            id[1] = registerValue[1];
            id[2] = registerValue[2];
            id[3] = 0xFE;
            id[4] = 0xFF;
            id[5] = registerValue[3];
            id[6] = registerValue[4];
            id[7] = registerValue[5];

            return id;
        }
    }
}

However if I add a lock like below:


using System;
using Microsoft.SPOT;
using Microsoft.SPOT.Hardware;

namespace Gateway.Utils
{
    public static class EPROMReader
    {
        private const byte ADDRESS = 0xA0 >> 1;
        private const byte MAC_ADDRESS_REG = 0xFA;
        static object _lock = new object();

        public static byte[] ReadMAC()
        {
            byte[] id = new byte[8];
            lock (_lock)
            {

                I2CDevice.Configuration config = new I2CDevice.Configuration(ADDRESS, 250);
                I2CDevice idDevice = new I2CDevice(config);
                I2CDevice.I2CTransaction[] i2cActions = new I2CDevice.I2CTransaction[2];

                byte[] registerNum = new byte[] { MAC_ADDRESS_REG };
                i2cActions[0] = I2CDevice.CreateWriteTransaction(registerNum);

                byte[] registerValue = new byte[6];
                i2cActions[1] = I2CDevice.CreateReadTransaction(registerValue);

                idDevice.Execute(i2cActions, 50);

                id[0] = registerValue[0];
                id[1] = registerValue[1];
                id[2] = registerValue[2];
                id[3] = 0xFE;
                id[4] = 0xFF;
                id[5] = registerValue[3];
                id[6] = registerValue[4];
                id[7] = registerValue[5];
            }
            return id;
        }
    }
}

I get a null reference exception on the _lock, which was surprising to me.
I thought it would get created when the class itself was being referenced.
The code calling the ReadMAC() is also static if thats of any help.

@ RandomNumber - Updating the SDK triggers that exception because we fixed a bug that prevented ReservePin (in the stack trace) from working in earlier SDKs.

On your second code, you shouldn’t need to recreate the I2CDevice object every time. I believe there is a Configuration property on it that you can use to dynamically change the config before every operation.

As to why lock throws an exception, it’s hard to say. Can you create a very minimal but complete program that still shows the issue?

@ John -
Ok! So is it correct to assume that the exception is thrown because of multiple attempts to reserve the same pin?

On your note that I shouldn’t have to recreate the I2CDevice, if I lift it out as following:


 public static class EPROMReader
 {
        private const byte ADDRESS = 0xA0 >> 1;
        private const byte MAC_ADDRESS_REG = 0xFA;

        private static I2CDevice.Configuration config = new I2CDevice.Configuration(ADDRESS, 250);
        private static I2CDevice idDevice = new I2CDevice(config);

        public static byte[] ReadMAC()
        {
            byte[] id = new byte[8];

            I2CDevice.I2CTransaction[] i2cActions = new I2CDevice.I2CTransaction[2];

            byte[] registerNum = new byte[] { MAC_ADDRESS_REG };
            i2cActions[0] = I2CDevice.CreateWriteTransaction(registerNum);

            byte[] registerValue = new byte[6];
            i2cActions[1] = I2CDevice.CreateReadTransaction(registerValue);

            idDevice.Execute(i2cActions, 50);

            id[0] = registerValue[0];
            id[1] = registerValue[1];
            id[2] = registerValue[2];
            id[3] = 0xFE;
            id[4] = 0xFF;
            id[5] = registerValue[3];
            id[6] = registerValue[4];
            id[7] = registerValue[5];

            return id;
        }
 }

I get the same issue as when I tried to add a lock, ie,

 
 idDevice.Execute(i2cActions, 50);

throws a NullReferenceException since idDevice does not get instatiated.

Unfortunately i cannot reproduce this behaviour in a smaller application. If I lift out the relevant parts, it works as I had expected. That is, the static objects get instantiated before they are used.

@ RandomNumber - It is correct to assume that for the first exception you got.

Using the latest firmware on our G120E Dev Board, I ran the below code and did not get a null reference exception. I did not change the EPROMReader class. It sounds like something else in your code is interfering.


using Microsoft.SPOT;
using Microsoft.SPOT.Hardware;

namespace MFConsoleApplication1 {
    public class Program {
        public static void Main() {
            var str = "";

            foreach (var i in EPROMReader.ReadMAC())
                str += i + " ";

            Debug.Print(str);
        }
    }

    public static class EPROMReader {
        private const byte ADDRESS = 0xA0 >> 1;
        private const byte MAC_ADDRESS_REG = 0xFA;

        private static I2CDevice.Configuration config = new I2CDevice.Configuration(ADDRESS, 250);
        private static I2CDevice idDevice = new I2CDevice(config);

        public static byte[] ReadMAC() {
            byte[] id = new byte[8];

            I2CDevice.I2CTransaction[] i2cActions = new I2CDevice.I2CTransaction[2];

            byte[] registerNum = new byte[] { MAC_ADDRESS_REG };
            i2cActions[0] = I2CDevice.CreateWriteTransaction(registerNum);

            byte[] registerValue = new byte[6];
            i2cActions[1] = I2CDevice.CreateReadTransaction(registerValue);

            idDevice.Execute(i2cActions, 50);

            id[0] = registerValue[0];
            id[1] = registerValue[1];
            id[2] = registerValue[2];
            id[3] = 0xFE;
            id[4] = 0xFF;
            id[5] = registerValue[3];
            id[6] = registerValue[4];
            id[7] = registerValue[5];

            return id;
        }
    }
}

I tried reimplementing it as a singleton so

private static I2CDevice instance;
public static I2CDevice Instance
{
        get
        {
            if(instance==null)
            {
                       var config = new I2CDevice.Configuration(ADDRESS, 250);
                       instance = new I2CDevice(config); 
            }
            return instance;
        }
}

And then at least the application starts. But I can’t make the singleton thread safe since adding a lock:

        private static I2CDevice instance;
        private static object _lock = new object();
        public static I2CDevice Instance
        {
            get
            {
                if(instance==null)
                {
                    lock(_lock)
                    {
                        if(instance==null)
                        {
                            var config = new I2CDevice.Configuration(ADDRESS, 250);
                            instance = new I2CDevice(config); 
                        }
                    }

                }
                return instance;
            }
        }

doesn’t work since _lock is null…

@ John -
Yes, I think so too, but I have no idea what. I’m not up to date on how static instantiation works in micro framework (not in full .net either) so unfortunately I don’t now where to start digging. I tried to move all relevant parts to a separate application but, like you, I dont get null reference exceptions then. So, it must be something else in my application that interfere whith when and how static objects are created.

@ RandomNumber - Try something like the below. The I2C objects will only be created once on construction. Make sure to construct the object somewhere early in your program.

I believe ReadMac is thread-safe only because I2CDevice.Execute locks internally. If not, you should be able to add a “lock (this.device)” right before the call to Execute.

I2CDevice does implement IDisposable so you may want to add Dispose logic depending on your program and the expected life-time of the EepromReader object.


public class EepromReader {
    private const byte Address = 0xA0 >> 1;
    private const byte MacAddressRegister = 0xFA;

    private byte[] buffer;
    private I2CDevice device;
    private I2CDevice.I2CTransaction[] transactions;

    public EepromReader() {
        this.buffer = new byte[6];

        this.device = new I2CDevice(new I2CDevice.Configuration(EepromReader.Address, 250));
        this.transactions = new I2CDevice.I2CTransaction[] {
            I2CDevice.CreateWriteTransaction(new byte[] { EepromReader.MacAddressRegister }),
            I2CDevice.CreateReadTransaction(this.buffer)
        };
    }

    public byte[] ReadMac() {
        var id = new byte[8];

        this.device.Execute(this.transactions, 50);

        id[0] = this.buffer[0];
        id[1] = this.buffer[1];
        id[2] = this.buffer[2];
        id[3] = 0xFE;
        id[4] = 0xFF;
        id[5] = this.buffer[3];
        id[6] = this.buffer[4];
        id[7] = this.buffer[5];

        return id;
    }
}

Ok, will try. But won’t this reintroduce risk of conflict if 2 instances of EepromReader are created at different parts in the application?

@ RandomNumber - Yes, but ideally you shouldn’t be creating two separate instances.

If that doesn’t work for you, the code you posted earlier with a lock surrounded by two ifs is the proper pattern I believe. If _lock is null, try setting it in a static constructor on the EPROMReader object instead of using a field initializer.

@ John -
First, thank you for helping me.

I would have preferred not to pass the object around, but it seems it might be my only option.

I tried to instantiate the lock in a static constructor instead,


  public sealed class EPROMReader
    {
        private const byte ADDRESS = 0xA0 >> 1;
        private const byte MAC_ADDRESS_REG = 0xFA;
        
        static EPROMReader()
        {
            _lock = new object();
        }

     
        private static I2CDevice instance;
        private static object _lock;
        public static I2CDevice Instance
        {
            get
            {
                if(instance==null)
                {
                    lock(_lock)
                    {
                        if(instance==null)
                        {
                            var config = new I2CDevice.Configuration(ADDRESS, 250);
                            instance = new I2CDevice(config); 
                        }
                    }

                }
                return instance;
            }
}

but i get the same error, lock is null.

Do you, or anyone else for that matter, have an idea what could be causing this behaviour?
(Or maybe that should be posted in a separate thread?)

@ RandomNumber - It really seems like something else in your application is setting _lock to null or corrupting it. The below code runs fine for me and _lock is not null. I would verify that the below program works for you in a new project. Then remove parts of your application until the lock works. The last removed part is what broke lock so you can narrow it down by removing smaller and smaller bits.


public class Program {
    private static object _lock = new object();

    public static void Main() {
        lock (_lock)
            Microsoft.SPOT.Debug.Print("Locked");
    }
}

I think I have been able to reproduce it now and I’m constructing a minimal application,
will post it shortly.

So it seems to be an issue with objects being instantiated depending on the name.
I found this thread: [url]https://www.ghielectronics.com/community/forum/topic?id=3063&page=1#msg61781[/url]
and started experimenting and could reproduce it in my own application. Here is a small application that illustrates the problem:

Main.cs:


using System;
using Microsoft.SPOT;
using A;

namespace StaticInit
{
    public class Program
    {
        public static void Main()
        {
            Debug.Print(Class1.string1);
        }
    }
}

Class1.cs:


using System;
using Microsoft.SPOT;
using Z;
namespace A
{
    public static class Class1
    {
       public static string string1 = Class2.GetString2();
    }
}

Class2.cs

using System;
using Microsoft.SPOT;

namespace Z
{
    public static class Class2
    {
        static object object1; 

        static Class2()
        {
            object1 = new object();
        }

        public static string GetString2()
        {
            if(object1 != null)
            {
                return "object1 was instantiated";
            }
            else
            {
                return "object1 is null!?!";
            }
        }

    }
}

Running that will print “object1 is null!?!”
but renaming namespaze A to Z and Z to A and then running will print “object1 was instantiated”
So, basically, static objects can’t be trusted to be instantiated in micro framework, which is kind of annoying.

@ RandomNumber - At this point then it may be easiest to use instances of the class without any static members.

@ John -
Indeed :slight_smile: