Merge pull request #50 from Annopaolo/dont-send-without-introspection

Do not send data before introspection during initialisation

device/device.go

@@ -21,6 +21,7 @@ import (
 	"os"
 	"path/filepath"
 	"runtime"
+	"sync"
 
 	"github.com/astarte-platform/astarte-go/client"
 	"github.com/astarte-platform/astarte-go/interfaces"
@@ -34,6 +35,11 @@ const (
 	DefaultInitialConnectionAttempts = 10
 )
 
+type messageQueue struct {
+	sync.Mutex
+	queue chan astarteMessageInfo
+}
+
 // Device is the base struct for Astarte Devices
 type Device struct {
 	deviceID                    string
@@ -43,7 +49,7 @@ type Device struct {
 	astarteAPIClient            *client.Client
 	brokerURL                   string
 	db                          *gorm.DB
-	messageQueue                chan astarteMessageInfo
+	inflightMessages            messageQueue
 	isSendingStoredMessages     bool
 	volatileMessages            []astarteMessageInfo
 	lastSentIntrospection       string
@@ -232,7 +238,11 @@ func (d *Device) Connect(result chan<- error) {
 		}
 
 		// Now that the client is up and running, we can start sending messages
-		d.messageQueue = make(chan astarteMessageInfo, d.opts.MaxInflightMessages)
+		d.inflightMessages.queue = make(chan astarteMessageInfo, d.opts.MaxInflightMessages)
+		// When initialized, mutexes are unlocked (see https://pkg.go.dev/sync#Mutex),
+		// so we lock it in order to allow publishing messages
+		// only if introspection has already been sent
+		d.inflightMessages.Lock()
 		go d.sendLoop()
 
 		// All good: notify, and our routine is over.

device/protocol_mqtt_v1.go

@@ -61,6 +61,8 @@ func (d *Device) initializeMQTTClient() error {
 	})
 
 	opts.SetConnectionLostHandler(func(client mqtt.Client, err error) {
+		// If connection is lost, we should stop dequeuing messages from the channel
+		d.inflightMessages.Lock()
 		if d.OnErrors != nil {
 			d.OnErrors(d, err)
 		}
@@ -257,6 +259,9 @@ func astarteOnConnectHandler(d *Device, sessionPresent bool) {
 		}
 	}
 
+	// Since control messages have been sent, we allow to send data
+	d.inflightMessages.Unlock()
+
 	// If some messages must be retried, do so
 	d.resendFailedMessages()
 
@@ -388,8 +393,10 @@ func (d *Device) UnsetProperty(interfaceName, path string) error {
 
 // The main publishing loop: retrieves messages from the publishing channel and sends them one at a time, in order
 func (d *Device) sendLoop() {
-	for next := range d.messageQueue {
-		d.publishMessage(next)
+	for {
+		d.inflightMessages.Lock()
+		d.publishMessage(<-d.inflightMessages.queue)
+		d.inflightMessages.Unlock()
 	}
 }
 
@@ -467,7 +474,7 @@ func (d *Device) enqueueRawMqttV1Message(astarteInterface interfaces.AstarteInte
 		fmt.Println("Sending previously stored messages with non-discard retention, the current message may be scheduled later")
 	}
 	message := makeAstarteMessageInfo(expiry, retention, astarteInterface.Name, interfacePath, astarteInterface.MajorVersion, qos, bsonPayload)
-	d.messageQueue <- message
+	d.inflightMessages.queue <- message
 
 	return nil
 }

device/store.go

@@ -117,7 +117,7 @@ func (d *Device) resendStoredMessages() {
 	for _, message := range messages {
 		if !isStoredMessageExpired(message) && !d.isInterfaceOutdatedInIntrospection(message.InterfaceName, message.InterfaceMajor) {
 			// if the message is not expired, try resending it
-			d.messageQueue <- message
+			d.inflightMessages.queue <- message
 		} else {
 			// else, it can be removed
 			d.removeFailedMessageFromStorage(message.StorageId)
@@ -135,7 +135,7 @@ func (d *Device) resendVolatileMessages() {
 		d.volatileMessages = d.volatileMessages[1:]
 		// try resending the message only if it is not expired
 		if !isStoredMessageExpired(message) && !d.isInterfaceOutdatedInIntrospection(message.InterfaceName, message.InterfaceMajor) {
-			d.messageQueue <- message
+			d.inflightMessages.queue <- message
 		}
 	}
 }