Improve compatibility checking

- added CompatibilityError to api package to represent error with
  list of incompatible messages
- added UnknownMsgError to adapter package to represent error for
  unknown message
- added list of registered message types

Change-Id: I2623b45135521d52612ba91e4605fc064a7fc76e
Signed-off-by: Ondrej Fabry <ofabry@cisco.com>

diff --git a/adapter/socketclient/socketclient.go b/adapter/socketclient/socketclient.go
index 043d253..366163f 100644 (file)
--- a/adapter/socketclient/socketclient.go
+++ b/adapter/socketclient/socketclient.go
@@ -419,7 +419,7 @@ func (c *vppClient) GetMsgID(msgName string, msgCrc string) (uint16, error) {
        msg := msgName + "_" + msgCrc
        msgID, ok := c.msgTable[msg]
        if !ok {
-               return 0, fmt.Errorf("unknown message: %q", msg)
+               return 0, &adapter.UnknownMsgError{msgName, msgCrc}
        }
        return msgID, nil
 }

diff --git a/adapter/vpp_api.go b/adapter/vpp_api.go
index 71bf7c2..b32f975 100644 (file)
--- a/adapter/vpp_api.go
+++ b/adapter/vpp_api.go
@@ -16,6 +16,7 @@ package adapter
 
 import (
        "errors"
+       "fmt"
 )
 
 const (
@@ -52,3 +53,14 @@ type VppAPI interface {
        // WaitReady waits until adapter is ready.
        WaitReady() error
 }
+
+// UnknownMsgError is the error type usually returned by GetMsgID
+// method of VppAPI. It describes the name and CRC for the unknown message.
+type UnknownMsgError struct {
+       MsgName string
+       MsgCrc  string
+}
+
+func (u *UnknownMsgError) Error() string {
+       return fmt.Sprintf("unknown message: %s_%s", u.MsgName, u.MsgCrc)
+}

diff --git a/adapter/vppapiclient/vppapiclient.go b/adapter/vppapiclient/vppapiclient.go
index fcd3f3f..977f32d 100644 (file)
--- a/adapter/vppapiclient/vppapiclient.go
+++ b/adapter/vppapiclient/vppapiclient.go
@@ -116,7 +116,7 @@ func (a *vppClient) GetMsgID(msgName string, msgCrc string) (uint16, error) {
        msgID := uint16(C.govpp_get_msg_index(nameAndCrc))
        if msgID == ^uint16(0) {
                // VPP does not know this message
-               return msgID, fmt.Errorf("unknown message: %v (crc: %v)", msgName, msgCrc)
+               return msgID, &adapter.UnknownMsgError{msgName, msgCrc}
        }
 
        return msgID, nil

diff --git a/api/binapi.go b/api/binapi.go
index 6495d0d..d933612 100644 (file)
--- a/api/binapi.go
+++ b/api/binapi.go
@@ -15,6 +15,8 @@
 package api
 
 import (
+       "fmt"
+       "reflect"
        "time"
 )
 
@@ -115,15 +117,32 @@ type SubscriptionCtx interface {
        Unsubscribe() error
 }
 
-var registeredMessages = make(map[string]Message)
+// CompatibilityError is the error type usually returned by CheckCompatibility
+// method of Channel. It describes all of the incompatible messages.
+type CompatibilityError struct {
+       // IncompatibleMessages is the list of all messages
+       // that failed compatibility check.
+       IncompatibleMessages []string
+}
+
+func (c *CompatibilityError) Error() string {
+       return fmt.Sprintf("%d incompatible messages: %v", len(c.IncompatibleMessages), c.IncompatibleMessages)
+}
+
+var (
+       registeredMessageTypes = make(map[reflect.Type]string)
+       registeredMessages     = make(map[string]Message)
+)
 
 // RegisterMessage is called from generated code to register message.
 func RegisterMessage(x Message, name string) {
-       name = x.GetMessageName() + "_" + x.GetCrcString()
-       /*if _, ok := registeredMessages[name]; ok {
-               panic(fmt.Errorf("govpp: duplicate message registered: %s (%s)", name, x.GetCrcString()))
-       }*/
-       registeredMessages[name] = x
+       typ := reflect.TypeOf(x)
+       namecrc := x.GetMessageName() + "_" + x.GetCrcString()
+       if _, ok := registeredMessageTypes[typ]; ok {
+               panic(fmt.Errorf("govpp: message type %v already registered as %s (%s)", typ, name, namecrc))
+       }
+       registeredMessages[namecrc] = x
+       registeredMessageTypes[typ] = name
 }
 
 // GetRegisteredMessages returns list of all registered messages.
@@ -131,6 +150,11 @@ func GetRegisteredMessages() map[string]Message {
        return registeredMessages
 }
 
+// GetRegisteredMessageTypes returns list of all registered message types.
+func GetRegisteredMessageTypes() map[reflect.Type]string {
+       return registeredMessageTypes
+}
+
 // GoVppAPIPackageIsVersion1 is referenced from generated binapi files
 // to assert that that code is compatible with this version of the GoVPP api package.
 const GoVppAPIPackageIsVersion1 = true

diff --git a/core/channel.go b/core/channel.go
index 5b513e3..363a267 100644 (file)
--- a/core/channel.go
+++ b/core/channel.go
@@ -23,6 +23,7 @@ import (
 
        "github.com/sirupsen/logrus"
 
+       "git.fd.io/govpp.git/adapter"
        "git.fd.io/govpp.git/api"
 )
 
@@ -144,14 +145,23 @@ func (ch *Channel) SendMultiRequest(msg api.Message) api.MultiRequestCtx {
 }
 
 func (ch *Channel) CheckCompatiblity(msgs ...api.Message) error {
+       var comperr api.CompatibilityError
        for _, msg := range msgs {
-               // TODO: collect all incompatible messages and return summarized error
                _, err := ch.msgIdentifier.GetMessageID(msg)
                if err != nil {
+                       if uerr, ok := err.(*adapter.UnknownMsgError); ok {
+                               m := fmt.Sprintf("%s_%s", uerr.MsgName, uerr.MsgCrc)
+                               comperr.IncompatibleMessages = append(comperr.IncompatibleMessages, m)
+                               continue
+                       }
+                       // other errors return immediatelly
                        return err
                }
        }
-       return nil
+       if len(comperr.IncompatibleMessages) == 0 {
+               return nil
+       }
+       return &comperr
 }
 
 func (ch *Channel) SubscribeNotification(notifChan chan api.Message, event api.Message) (api.SubscriptionCtx, error) {

diff --git a/core/channel_test.go b/core/channel_test.go
index 7e721cd..849255a 100644 (file)
--- a/core/channel_test.go
+++ b/core/channel_test.go
@@ -213,15 +213,15 @@ func TestSetReplyTimeoutMultiRequest(t *testing.T) {
        ctx.mockVpp.MockReply(
                &interfaces.SwInterfaceDetails{
                        SwIfIndex:     1,
-                       InterfaceName: []byte("if-name-test"),
+                       InterfaceName: "if-name-test",
                },
                &interfaces.SwInterfaceDetails{
                        SwIfIndex:     2,
-                       InterfaceName: []byte("if-name-test"),
+                       InterfaceName: "if-name-test",
                },
                &interfaces.SwInterfaceDetails{
                        SwIfIndex:     3,
-                       InterfaceName: []byte("if-name-test"),
+                       InterfaceName: "if-name-test",
                },
        )
        ctx.mockVpp.MockReply(&ControlPingReply{})
@@ -288,7 +288,7 @@ func TestMultiRequestDouble(t *testing.T) {
                msgs = append(msgs, mock.MsgWithContext{
                        Msg: &interfaces.SwInterfaceDetails{
                                SwIfIndex:     uint32(i),
-                               InterfaceName: []byte("if-name-test"),
+                               InterfaceName: "if-name-test",
                        },
                        Multipart: true,
                        SeqNum:    1,
@@ -301,7 +301,7 @@ func TestMultiRequestDouble(t *testing.T) {
                        mock.MsgWithContext{
                                Msg: &interfaces.SwInterfaceDetails{
                                        SwIfIndex:     uint32(i),
-                                       InterfaceName: []byte("if-name-test"),
+                                       InterfaceName: "if-name-test",
                                },
                                Multipart: true,
                                SeqNum:    2,
@@ -427,7 +427,7 @@ func TestReceiveReplyAfterTimeoutMultiRequest(t *testing.T) {
                msgs = append(msgs, mock.MsgWithContext{
                        Msg: &interfaces.SwInterfaceDetails{
                                SwIfIndex:     uint32(i),
-                               InterfaceName: []byte("if-name-test"),
+                               InterfaceName: "if-name-test",
                        },
                        Multipart: true,
                        SeqNum:    2,

diff --git a/examples/rpc-service/rpc_service.go b/examples/rpc-service/rpc_service.go
index ca0c295..8ff6c08 100644 (file)
--- a/examples/rpc-service/rpc_service.go
+++ b/examples/rpc-service/rpc_service.go
@@ -17,12 +17,12 @@
 package main
 
 import (
-       "bytes"
        "context"
        "flag"
        "fmt"
        "io"
        "log"
+       "strings"
 
        "git.fd.io/govpp.git"
        "git.fd.io/govpp.git/adapter/socketclient"
@@ -88,6 +88,6 @@ func interfaceDump(ch api.Channel) {
                if err != nil {
                        log.Fatalln("ERROR: DumpSwInterface failed:", err)
                }
-               fmt.Printf("- interface: %s\n", bytes.Trim(iface.InterfaceName, "\x00"))
+               fmt.Printf("- interface: %s\n", strings.Trim(iface.InterfaceName, "\x00"))
        }
 }

diff --git a/examples/simple-client/simple_client.go b/examples/simple-client/simple_client.go
index 3a24e6a..3ed811d 100644 (file)
--- a/examples/simple-client/simple_client.go
+++ b/examples/simple-client/simple_client.go
@@ -65,6 +65,10 @@ func main() {
 
        vppVersion(ch)
 
+       if err := ch.CheckCompatiblity(interfaces.AllMessages()...); err != nil {
+               log.Fatal(err)
+       }
+
        createLoopback(ch)
        createLoopback(ch)
        interfaceDump(ch)