[VOL-4164] openonu-adapter-go voltctl device onuimage list extend OMCI-attribute checks and update internal data

Change-Id: I67ad43cbc984633edffbd24fa4f4ff7436c840d3
diff --git a/internal/pkg/onuadaptercore/mib_sync.go b/internal/pkg/onuadaptercore/mib_sync.go
index efb6e69..8104d51 100644
--- a/internal/pkg/onuadaptercore/mib_sync.go
+++ b/internal/pkg/onuadaptercore/mib_sync.go
@@ -212,11 +212,14 @@
 
 func (oo *OnuDeviceEntry) enterGettingMibTemplateState(ctx context.Context, e *fsm.Event) {
 
+	oo.mutexOnuSwImageIndications.RLock()
 	if oo.onuSwImageIndications.activeEntityEntry.valid {
 		oo.mutexPersOnuConfig.Lock()
 		oo.sOnuPersistentData.PersActiveSwVersion = oo.onuSwImageIndications.activeEntityEntry.version
 		oo.mutexPersOnuConfig.Unlock()
+		oo.mutexOnuSwImageIndications.RUnlock()
 	} else {
+		oo.mutexOnuSwImageIndications.RUnlock()
 		logger.Errorw(ctx, "get-mib-template: no active SW version found, working with empty SW version, which might be untrustworthy",
 			log.Fields{"device-id": oo.deviceID})
 	}
@@ -696,6 +699,7 @@
 	oo.mutexPersOnuConfig.RUnlock()
 	if firstSwImageMeID == entityID {
 		//always accept the state of the first image (2nd image info should not yet be available)
+		oo.mutexOnuSwImageIndications.Lock()
 		if imageIsActive == swIsActive {
 			oo.onuSwImageIndications.activeEntityEntry.entityID = entityID
 			oo.onuSwImageIndications.activeEntityEntry.valid = true
@@ -715,10 +719,12 @@
 			//  (state of the second image is always expected afterwards or just invalid)
 			oo.onuSwImageIndications.activeEntityEntry.valid = false
 		}
+		oo.mutexOnuSwImageIndications.Unlock()
 		_ = oo.pMibUploadFsm.pFsm.Event(ulEvGetSecondSwVersion)
 		return
 	} else if secondSwImageMeID == entityID {
 		//2nd image info might conflict with first image info, in which case we priorize first image info!
+		oo.mutexOnuSwImageIndications.Lock()
 		if imageIsActive == swIsActive { //2nd image reported to be active
 			if oo.onuSwImageIndications.activeEntityEntry.valid {
 				//conflict exists - state of first image is left active
@@ -751,6 +757,7 @@
 			oo.onuSwImageIndications.inactiveEntityEntry.version = imageVersion
 			oo.onuSwImageIndications.inactiveEntityEntry.isCommitted = imageIsCommitted
 		}
+		oo.mutexOnuSwImageIndications.Unlock()
 		_ = oo.pMibUploadFsm.pFsm.Event(ulEvGetMacAddress)
 		return
 	}
@@ -979,29 +986,40 @@
 
 //GetActiveImageMeID returns the Omci MeId of the active ONU image together with error code for validity
 func (oo *OnuDeviceEntry) GetActiveImageMeID(ctx context.Context) (uint16, error) {
+	oo.mutexOnuSwImageIndications.RLock()
 	if oo.onuSwImageIndications.activeEntityEntry.valid {
-		return oo.onuSwImageIndications.activeEntityEntry.entityID, nil
+		value := oo.onuSwImageIndications.activeEntityEntry.entityID
+		oo.mutexOnuSwImageIndications.RUnlock()
+		return value, nil
 	}
+	oo.mutexOnuSwImageIndications.RUnlock()
 	return 0xFFFF, fmt.Errorf("no valid active image found: %s", oo.deviceID)
 }
 
 //GetInactiveImageMeID returns the Omci MeId of the inactive ONU image together with error code for validity
 func (oo *OnuDeviceEntry) GetInactiveImageMeID(ctx context.Context) (uint16, error) {
+	oo.mutexOnuSwImageIndications.RLock()
 	if oo.onuSwImageIndications.inactiveEntityEntry.valid {
-		return oo.onuSwImageIndications.inactiveEntityEntry.entityID, nil
+		value := oo.onuSwImageIndications.inactiveEntityEntry.entityID
+		oo.mutexOnuSwImageIndications.RUnlock()
+		return value, nil
 	}
+	oo.mutexOnuSwImageIndications.RUnlock()
 	return 0xFFFF, fmt.Errorf("no valid inactive image found: %s", oo.deviceID)
 }
 
 //IsImageToBeCommitted returns true if the active image is still uncommitted
 func (oo *OnuDeviceEntry) IsImageToBeCommitted(ctx context.Context, aImageID uint16) bool {
+	oo.mutexOnuSwImageIndications.RLock()
 	if oo.onuSwImageIndications.activeEntityEntry.valid {
 		if oo.onuSwImageIndications.activeEntityEntry.entityID == aImageID {
 			if oo.onuSwImageIndications.activeEntityEntry.isCommitted == swIsUncommitted {
+				oo.mutexOnuSwImageIndications.RUnlock()
 				return true
 			}
 		}
 	}
+	oo.mutexOnuSwImageIndications.RUnlock()
 	return false //all other case are treated as 'nothing to commit
 }
 func (oo *OnuDeviceEntry) getMibFromTemplate(ctx context.Context) bool {
diff --git a/internal/pkg/onuadaptercore/onu_device_entry.go b/internal/pkg/onuadaptercore/onu_device_entry.go
index 0b2a218..6302bc7 100644
--- a/internal/pkg/onuadaptercore/onu_device_entry.go
+++ b/internal/pkg/onuadaptercore/onu_device_entry.go
@@ -266,26 +266,27 @@
 
 // OnuDeviceEntry - ONU device info and FSM events.
 type OnuDeviceEntry struct {
-	deviceID                  string
-	baseDeviceHandler         *deviceHandler
-	pOpenOnuAc                *OpenONUAC
-	coreProxy                 adapterif.CoreProxy
-	adapterProxy              adapterif.AdapterProxy
-	PDevOmciCC                *omciCC
-	pOnuDB                    *onuDeviceDB
-	mibTemplateKVStore        *db.Backend
-	mutexPersOnuConfig        sync.RWMutex
-	sOnuPersistentData        onuPersistentData
-	mibTemplatePath           string
-	mutexOnuKVStore           sync.RWMutex
-	onuKVStore                *db.Backend
-	onuKVStorePath            string
-	mutexOnuKVStoreProcResult sync.RWMutex
-	onuKVStoreProcResult      error //error indication of processing
-	chOnuKvProcessingStep     chan uint8
-	onuSwImageIndications     sSwImageIndications
-	mutexOnuImageStatus       sync.RWMutex
-	pOnuImageStatus           *OnuImageStatus
+	deviceID                   string
+	baseDeviceHandler          *deviceHandler
+	pOpenOnuAc                 *OpenONUAC
+	coreProxy                  adapterif.CoreProxy
+	adapterProxy               adapterif.AdapterProxy
+	PDevOmciCC                 *omciCC
+	pOnuDB                     *onuDeviceDB
+	mibTemplateKVStore         *db.Backend
+	mutexPersOnuConfig         sync.RWMutex
+	sOnuPersistentData         onuPersistentData
+	mibTemplatePath            string
+	mutexOnuKVStore            sync.RWMutex
+	onuKVStore                 *db.Backend
+	onuKVStorePath             string
+	mutexOnuKVStoreProcResult  sync.RWMutex
+	onuKVStoreProcResult       error //error indication of processing
+	chOnuKvProcessingStep      chan uint8
+	mutexOnuSwImageIndications sync.RWMutex
+	onuSwImageIndications      sSwImageIndications
+	mutexOnuImageStatus        sync.RWMutex
+	pOnuImageStatus            *OnuImageStatus
 	//lockDeviceEntries           sync.RWMutex
 	mibDbClass    func(context.Context) error
 	supportedFsms OmciDeviceFsms
@@ -893,17 +894,25 @@
 }
 
 func (oo *OnuDeviceEntry) getActiveImageVersion(ctx context.Context) string {
+	oo.mutexOnuSwImageIndications.RLock()
 	if oo.onuSwImageIndications.activeEntityEntry.valid {
-		return oo.onuSwImageIndications.activeEntityEntry.version
+		value := oo.onuSwImageIndications.activeEntityEntry.version
+		oo.mutexOnuSwImageIndications.RUnlock()
+		return value
 	}
+	oo.mutexOnuSwImageIndications.RUnlock()
 	logger.Debugw(ctx, "Active Image is not valid", log.Fields{"device-id": oo.deviceID})
 	return ""
 }
 
 func (oo *OnuDeviceEntry) getInactiveImageVersion(ctx context.Context) string {
+	oo.mutexOnuSwImageIndications.RLock()
 	if oo.onuSwImageIndications.inactiveEntityEntry.valid {
-		return oo.onuSwImageIndications.inactiveEntityEntry.version
+		value := oo.onuSwImageIndications.inactiveEntityEntry.version
+		oo.mutexOnuSwImageIndications.RUnlock()
+		return value
 	}
+	oo.mutexOnuSwImageIndications.RUnlock()
 	logger.Debugw(ctx, "Inactive Image is not valid", log.Fields{"device-id": oo.deviceID})
 	return ""
 }
diff --git a/internal/pkg/onuadaptercore/onu_image_status.go b/internal/pkg/onuadaptercore/onu_image_status.go
index fb34a26..5e610ac 100755
--- a/internal/pkg/onuadaptercore/onu_image_status.go
+++ b/internal/pkg/onuadaptercore/onu_image_status.go
@@ -50,6 +50,7 @@
 	cImgProductCode = "ProductCode"
 	cImgImageHash   = "ImageHash"
 )
+const cResponse = "response: "
 
 //NewOnuImageStatus creates a new instance of OnuImageStatus
 func NewOnuImageStatus(pDevEntry *OnuDeviceEntry) *OnuImageStatus {
@@ -61,14 +62,19 @@
 		respChannel:         make(chan Message),
 	}
 }
+
 func (oo *OnuImageStatus) getOnuImageStatus(ctx context.Context) (*voltha.OnuImages, error) {
 
-	var images voltha.OnuImages
-
+	if !oo.pDevEntry.baseDeviceHandler.isReadyForOmciConfig() {
+		logger.Errorw(ctx, "command rejected - improper device state", log.Fields{"device-id": oo.deviceID})
+		return nil, fmt.Errorf("command-rejected-improper-device-state")
+	}
 	if oo.pDevEntry.PDevOmciCC == nil {
 		logger.Errorw(ctx, "omciCC not ready to receive omci messages", log.Fields{"device-id": oo.deviceID})
 		return nil, fmt.Errorf("omciCC-not-ready-to-receive-omci-messages")
 	}
+	var images voltha.OnuImages
+
 	for i := firstSwImageMeID; i <= secondSwImageMeID; i++ {
 		logger.Debugw(ctx, "getOnuImageStatus for image id", log.Fields{"image-id": i, "device-id": oo.deviceID})
 
@@ -97,6 +103,8 @@
 		images.Items = append(images.Items, &image)
 	}
 	logger.Debugw(ctx, "images of the ONU", log.Fields{"images": images})
+	oo.updateOnuSwImageIndications(ctx, &images)
+	oo.updateOnuSwImagePersistentData(ctx)
 	return &images, nil
 }
 
@@ -181,38 +189,9 @@
 		if msgObj.EntityClass == oo.pLastTxMeInstance.GetClassID() &&
 			msgObj.EntityInstance == oo.pLastTxMeInstance.GetEntityID() {
 			oo.mutexPLastTxMeInstance.RUnlock()
-
-			meAttributes := msgObj.Attributes
-			logger.Debugw(ctx, "processGetOnuImageStatusResp omci attributes received", log.Fields{"attributes": meAttributes, "device-id": oo.deviceID})
-
-			for k := range oo.requestedAttributes {
-				switch k {
-				case cImgIsCommitted:
-					if meAttributes[cImgIsCommitted].(uint8) == swIsCommitted {
-						image.IsCommited = true
-					} else {
-						image.IsCommited = false
-					}
-				case cImgIsActive:
-					if meAttributes[cImgIsActive].(uint8) == swIsActive {
-						image.IsActive = true
-					} else {
-						image.IsActive = false
-					}
-				case cImgIsValid:
-					if meAttributes[cImgIsValid].(uint8) == swIsValid {
-						image.IsValid = true
-					} else {
-						image.IsValid = false
-					}
-				case cImgVersion:
-					image.Version = TrimStringFromMeOctet(meAttributes[cImgVersion])
-				case cImgProductCode:
-					image.ProductCode = TrimStringFromMeOctet(meAttributes[cImgProductCode])
-				case cImgImageHash:
-					bytes, _ := me.InterfaceToOctets(meAttributes[cImgImageHash])
-					image.Hash = hex.EncodeToString(bytes)
-				}
+			if err := oo.processAttributesReceived(ctx, msgObj, image); err != nil {
+				logger.Errorw(ctx, err.Error(), log.Fields{"device-id": oo.deviceID})
+				return err
 			}
 			return nil
 		}
@@ -224,11 +203,134 @@
 	logger.Errorw(ctx, "processGetOnuImageStatusResp pLastTxMeInstance is nil", log.Fields{"device-id": oo.deviceID})
 	return fmt.Errorf("process-image-status-response-error")
 }
+
+func (oo *OnuImageStatus) processAttributesReceived(ctx context.Context, msgObj *omci.GetResponse, image *voltha.OnuImage) error {
+	meAttributes := msgObj.Attributes
+	logger.Debugw(ctx, "processAttributesReceived", log.Fields{"attributes": meAttributes, "device-id": oo.deviceID})
+
+	for k := range oo.requestedAttributes {
+
+		if msgObj.Result != me.Success && k != cImgProductCode && k != cImgImageHash {
+			logger.Errorw(ctx, "processAttributesReceived retrieval of mandatory attributes failed",
+				log.Fields{"device-id": oo.deviceID})
+			return fmt.Errorf("process-image-status-response-error")
+		}
+		switch k {
+
+		// mandatory attributes
+		case cImgIsCommitted:
+			if meAttributes[cImgIsCommitted].(uint8) == swIsCommitted {
+				image.IsCommited = true
+			} else {
+				image.IsCommited = false
+			}
+		case cImgIsActive:
+			if meAttributes[cImgIsActive].(uint8) == swIsActive {
+				image.IsActive = true
+			} else {
+				image.IsActive = false
+			}
+		case cImgIsValid:
+			if meAttributes[cImgIsValid].(uint8) == swIsValid {
+				image.IsValid = true
+			} else {
+				image.IsValid = false
+			}
+		case cImgVersion:
+			image.Version = TrimStringFromMeOctet(meAttributes[cImgVersion])
+
+		// optional attributes
+		case cImgProductCode:
+			if msgObj.Result == me.Success {
+				image.ProductCode = TrimStringFromMeOctet(meAttributes[cImgProductCode])
+			} else {
+				sResult := msgObj.Result.String()
+				logger.Infow(ctx, "processAttributesReceived - ProductCode",
+					log.Fields{"result": sResult, "unsupported attribute mask": msgObj.UnsupportedAttributeMask, "device-id": oo.deviceID})
+				image.ProductCode = cResponse + sResult
+			}
+		case cImgImageHash:
+			if msgObj.Result == me.Success {
+				bytes, _ := me.InterfaceToOctets(meAttributes[cImgImageHash])
+				image.Hash = hex.EncodeToString(bytes)
+			} else {
+				sResult := msgObj.Result.String()
+				logger.Infow(ctx, "processAttributesReceived - ImageHash",
+					log.Fields{"result": sResult, "unsupported attribute mask": msgObj.UnsupportedAttributeMask, "device-id": oo.deviceID})
+				image.Hash = cResponse + sResult
+			}
+		}
+	}
+	return nil
+}
+
+func (oo *OnuImageStatus) updateOnuSwImageIndications(ctx context.Context, images *voltha.OnuImages) {
+
+	oo.pDevEntry.mutexOnuSwImageIndications.Lock()
+	validActiveImageFound := false
+	for i := firstSwImageMeID; i <= secondSwImageMeID; i++ {
+		if images.Items[i].IsActive && images.Items[i].IsValid {
+			oo.pDevEntry.onuSwImageIndications.activeEntityEntry.entityID = uint16(i)
+			oo.pDevEntry.onuSwImageIndications.activeEntityEntry.valid = images.Items[i].IsValid
+			oo.pDevEntry.onuSwImageIndications.activeEntityEntry.version = images.Items[i].Version
+			if images.Items[i].IsCommited {
+				oo.pDevEntry.onuSwImageIndications.activeEntityEntry.isCommitted = swIsCommitted
+			} else {
+				oo.pDevEntry.onuSwImageIndications.activeEntityEntry.isCommitted = swIsUncommitted
+			}
+			validActiveImageFound = true
+			break
+		}
+	}
+	if !validActiveImageFound {
+		oo.pDevEntry.onuSwImageIndications.activeEntityEntry.valid = false
+	}
+	validInactiveImageFound := false
+	for i := firstSwImageMeID; i <= secondSwImageMeID; i++ {
+		if !images.Items[i].IsActive && images.Items[i].IsValid {
+			oo.pDevEntry.onuSwImageIndications.inactiveEntityEntry.entityID = uint16(i)
+			oo.pDevEntry.onuSwImageIndications.inactiveEntityEntry.valid = images.Items[i].IsValid
+			oo.pDevEntry.onuSwImageIndications.inactiveEntityEntry.version = images.Items[i].Version
+			if images.Items[i].IsCommited {
+				oo.pDevEntry.onuSwImageIndications.inactiveEntityEntry.isCommitted = swIsCommitted
+			} else {
+				oo.pDevEntry.onuSwImageIndications.inactiveEntityEntry.isCommitted = swIsUncommitted
+			}
+			validInactiveImageFound = true
+			break
+		}
+	}
+	if !validInactiveImageFound {
+		oo.pDevEntry.onuSwImageIndications.inactiveEntityEntry.valid = false
+	}
+	oo.pDevEntry.mutexOnuSwImageIndications.Unlock()
+}
+
+func (oo *OnuImageStatus) updateOnuSwImagePersistentData(ctx context.Context) {
+
+	activeImageVersion := oo.pDevEntry.getActiveImageVersion(ctx)
+	oo.pDevEntry.mutexPersOnuConfig.Lock()
+	if oo.pDevEntry.sOnuPersistentData.PersActiveSwVersion != activeImageVersion {
+		logger.Infow(ctx, "Active SW version has been changed at ONU - update persistent data",
+			log.Fields{"old version": oo.pDevEntry.sOnuPersistentData.PersActiveSwVersion,
+				"new version": activeImageVersion, "device-id": oo.deviceID})
+		oo.pDevEntry.sOnuPersistentData.PersActiveSwVersion = activeImageVersion
+		oo.pDevEntry.mutexPersOnuConfig.Unlock()
+		if err := oo.pDevEntry.baseDeviceHandler.storePersistentData(ctx); err != nil {
+			logger.Warnw(ctx, "store persistent data error - continue for now as there will be additional write attempts",
+				log.Fields{"device-id": oo.deviceID, "err": err})
+		}
+		return
+	}
+	oo.pDevEntry.mutexPersOnuConfig.Unlock()
+}
+
 func (oo *OnuImageStatus) setWaitingForResp(value bool) {
 	oo.mutexWaitingForResp.Lock()
 	oo.waitingForResp = value
 	oo.mutexWaitingForResp.Unlock()
 }
+
 func (oo *OnuImageStatus) isWaitingForResp() bool {
 	oo.mutexWaitingForResp.RLock()
 	value := oo.waitingForResp