Main Site Documentation

New, better CAN implementation?


#1

I‘ve been working with CAN since NETMF 4.1, and I‘ve ran into numerous issues with it. I think we need a new, better CAN implementation from GHI :slight_smile: So, here are my suggestions:

  1. Initialization shouldn‘t start raising events immediately. That leads to all kinds of issues I‘m fighting ever since I‘ve started coding NETMF. For example:
    a. If I dont‘t attach an event handler to CanDataReceived event immediately after calling constructor, G400 has problems with high traffic, as it stacks incoming messages somewhere internally until all memory goes to BINARY_BLOB_HEAD and G400 locks up before even the program starts normally. EMX, on the other hand, isn‘t raising events at all until it receives 127 messages — which is problematic in low traffic conditions.
    b. Sometimes I want to turn CAN off for a while, but once it‘s on there‘s no way to stop it. Especially when I stop the program for debugging and peripherals continue working, I end up with thousands of RXOVER errors that paralyzes the program once I continue its execution.
    c. There are some smaller issues that I will skip to save typing.

So I think constructor should only set the rates and stuff, and Enable()/Disable() functions should set the state of the CAN. Would be very handy, in my opinion.

  1. PostMessages() function is very misleading. No matter the count argument passed to it, it only sends one message. I don‘t know if it‘s a bug or a hardware limitation, but the same happens on EMX/G120/G400, so I think it should either be fixed or renamed to PostMessage(). Better to have both. I want to send only a single message quite often, but I have to construct an array of messages with one element in it to do so.
  2. Initialize() is misleading, too. It accepts bitrate argument, which is actually a register of the chip, but it‘s not written anywhere. It‘s not bitrate, it‘s something that is different for every platform. It would be good to have more Initialize() overloads, one might accept predefined enumeration of possible CAN bus speeds, other be left as it is for those who have specific needs (note camel casing in enums):
var can=new Can(CAN.Channel.Channel_1, G400CanSpeed.125k)
var can=new Can(CAN.Channel.Channel_1, EmxCanSpeed.125k)
var can=new Can(CAN.Channel.Channel_1, 0x7F1235AA)
  1. The whole implementation should follow official Framework Design Guidelines more strictly (like CANDataReceivedArgs should become CanDataReceivedArgs and so on).

Here’s some scaffolding that illustrates my points:

  
//==== Usage ====

   //Creating a CAN object
            var myCan = new Can(CanChannel.One, CanSpeed.EmxAt250k);

            //Attaching event handler (CAN is not running yet!)
            myCan.MessagesReceived += (sender, e) => {
                  Debug.Print("Something arrived!");
            };

            //Enabling can, it can now send and receive
            myCan.Enable();
            Debug.Print(myCan.IsEnabled.ToString()); //should print: true

            for (int i = 0; i < 10; i++) {
                //Fetching some messages from CAN. Important: it should never return null, if there's no messages, then an empty array should be returned
                var receivedMessages = myCan.GetMessages();
                Debug.Print("Number of received messages: "+ receivedMessages.Length.ToString());
                Thread.Sleep(100);
            }

            //Posting a single message (it's quite a frequent operation for me. I don't always want to send a batch of messages
            myCan.PostMessage(new CanMessage(12, 4, new byte[] { 1, 2, 3, 4 }));

            //Posting a batch of messages
            var messagesToPost = new CanMessage[5];
            myCan.PostMessages(messagesToPost);

            //Posting a selected range
            myCan.PostMessages(messagesToPost, 1, 3);

            //Disabling can, it won't receive or send
            myCan.Disable();
            Debug.Print(myCan.IsEnabled.ToString()); //should print: false
 

//==== Classes ====
  public class CanMessage {
        public Int32 Id { get; set; }
        public Int32 DataLength { get; set; }
        public byte[] Data { get; set; }

        public CanMessage(Int32 id, Int32 DataLength, byte[] data) { }
    }

    public class Can {
        public Can(CanChannel channel, CanSpeed speed) { }
        public Can(CanChannel channel, UInt32 speedRegisterValue) { }

        public void Enable() { }
        public void Disable() { }

        public void PostMessages(CanMessage[] messagesToPost) { }
        public void PostMessages(CanMessage[] messagesToPost, Int32 offset, Int32 count) { }
        public void PostMessage(CanMessage messageToPost) { }
        public CanMessage[] GetMessages() { return new CanMessage[1]; }

        public Boolean IsEnabled { get; set; }

        public event MessagesReceivedEventHandler MessagesReceived;

    }

    public delegate void MessagesReceivedEventHandler(Can sender, MessagesReceivedEventArgs e);
    public class MessagesReceivedEventArgs { }

    public enum CanChannel {
        One,
        Two
    }

    public enum CanSpeed {
        G400At100k,
        G400At200k,
        EmxAt250k,
        G120At1000k
    }

I think it follows the .NET spirit better than the current implementation (please don’t beat me).


#2

We will look into all these on the next major release since things may not be backward compatible. Thanks for the feedback.


#3

Simon,

I’m using CAN on Panda II boards and also run into the issue that PostMessages only posts one message. I wish I would have come across your post earlier - it took me hours of debugging to find that one out.
I have to add a Sleep(1) between every post to get is to work properly. That’s quite unsettling because I wonder if “1” will work in all circumstances ? Maybe 2 or 3 is safer ?

Another issue (at least on the PandaII) I came across is that the CAN bus needs to be reset after every RX error. Not a big issue once you know about it, but not an easy one to find out (never happened on my desk, but a lot in the field) and rather stange in my opinion.

Stefaan


#4

I’m not sure 100%, but I think this comes from CAN specification.


#5

It doesn’t necessarily has to be backwards compatible. It may just be another implementation, coexisting with the old one. Microsoft did that in the big .NET a few times, so why can’t you? :slight_smile: