VOL-1439: Fixes for proper table attribute handling
during MIB audit/resynchronization.  Also includes a fix to
properly count MIB-DATA-SYNC increments on sets and software-download
operations

Having to touch files since adtran unit tests and others are a bit flaky

Change-Id: I30a343aae91d5bcac56d068a37c18b29265d3bd9
diff --git a/voltha/extensions/omci/omci_cc.py b/voltha/extensions/omci/omci_cc.py
index 29711ef..7d0874c 100644
--- a/voltha/extensions/omci/omci_cc.py
+++ b/voltha/extensions/omci/omci_cc.py
@@ -36,9 +36,9 @@
     return ''.join('%02x' % ord(c) for c in buffer)
 
 
-DEFAULT_OMCI_TIMEOUT = 3                           # Seconds
-MAX_OMCI_REQUEST_AGE = 60                          # Seconds
-DEFAULT_OMCI_DOWNLOAD_SECTION_SIZE = 31            # Bytes
+DEFAULT_OMCI_TIMEOUT = 3                         # Seconds
+MAX_OMCI_REQUEST_AGE = 60                        # Seconds
+DEFAULT_OMCI_DOWNLOAD_SECTION_SIZE = 31          # Bytes
 
 CONNECTED_KEY = 'connected'
 TX_REQUEST_KEY = 'tx-request'
@@ -58,7 +58,12 @@
     MIB_Reset = 8,
     Connectivity = 9,
     Get_ALARM_Get = 10,
-    Get_ALARM_Get_Next = 11
+    Get_ALARM_Get_Next = 11,
+    Start_Software_Download = 12,
+    Download_Section = 13,
+    End_Software_Download = 14,
+    Activate_Software = 15,
+    Commit_Software = 15,
 
 
 # abbreviations
@@ -69,8 +74,8 @@
 class OMCI_CC(object):
     """ Handle OMCI Communication Channel specifics for Adtran ONUs"""
 
-    MIN_OMCI_TX_ID_LOW_PRIORITY = 0x0001  # 2 Octets max
-    MAX_OMCI_TX_ID_LOW_PRIORITY = 0x7FFF  # 2 Octets max
+    MIN_OMCI_TX_ID_LOW_PRIORITY = 0x0001   # 2 Octets max
+    MAX_OMCI_TX_ID_LOW_PRIORITY = 0x7FFF   # 2 Octets max
     MIN_OMCI_TX_ID_HIGH_PRIORITY = 0x8000  # 2 Octets max
     MAX_OMCI_TX_ID_HIGH_PRIORITY = 0xFFFF  # 2 Octets max
     LOW_PRIORITY = 0
diff --git a/voltha/extensions/omci/state_machines/mib_sync.py b/voltha/extensions/omci/state_machines/mib_sync.py
index 1a61335..1b7c7ba 100644
--- a/voltha/extensions/omci/state_machines/mib_sync.py
+++ b/voltha/extensions/omci/state_machines/mib_sync.py
@@ -72,9 +72,9 @@
         # Do wildcard 'stop' trigger last so it covers all previous states
         {'trigger': 'stop', 'source': '*', 'dest': 'disabled'},
     ]
-    DEFAULT_TIMEOUT_RETRY = 5      # Seconds to delay after task failure/timeout
-    DEFAULT_AUDIT_DELAY = 60       # Periodic tick to audit the MIB Data Sync
-    DEFAULT_RESYNC_DELAY = 300     # Periodically force a resync
+    DEFAULT_TIMEOUT_RETRY = 5     # Seconds to delay after task failure/timeout
+    DEFAULT_AUDIT_DELAY = 60      # Periodic tick to audit the MIB Data Sync
+    DEFAULT_RESYNC_DELAY = 300    # Periodically force a resync
 
     def __init__(self, agent, device_id, mib_sync_tasks, db,
                  advertise_events=False,
@@ -143,6 +143,10 @@
             RxEvent.Create: None,
             RxEvent.Delete: None,
             RxEvent.Set: None,
+            RxEvent.Start_Software_Download: None,
+            RxEvent.End_Software_Download: None,
+            RxEvent.Activate_Software: None,
+            RxEvent.Commit_Software: None,
         }
         self._omci_cc_sub_mapping = {
             RxEvent.MIB_Reset: self.on_mib_reset_response,
@@ -152,6 +156,10 @@
             RxEvent.Create: self.on_create_response,
             RxEvent.Delete: self.on_delete_response,
             RxEvent.Set: self.on_set_response,
+            RxEvent.Start_Software_Download: self.on_software_event,
+            RxEvent.End_Software_Download: self.on_software_event,
+            RxEvent.Activate_Software: self.on_software_event,
+            RxEvent.Commit_Software: self.on_software_event,
         }
         self._onu_dev_subscriptions = {               # DevEvent.enum -> Subscription Object
             DevEvent.OmciCapabilitiesEvent: None
@@ -216,6 +224,7 @@
         if self._database is not None:
             self._database.save_mib_data_sync(self._device_id,
                                               self._mib_data_sync)
+            self.log.info("mds-updated", device=self._device_id, mds=self._mib_data_sync)
 
     @property
     def last_mib_db_sync(self):
@@ -522,8 +531,9 @@
             self._attr_diffs = attr_diffs if attr_diffs and len(attr_diffs) else None
             self._audited_olt_db = olt_db
             self._audited_onu_db = onu_db
+            audited_mds = self._audited_onu_db[MDS_KEY]
 
-            mds_equal = self.mib_data_sync == self._audited_onu_db[MDS_KEY]
+            mds_equal = self.mib_data_sync == audited_mds
 
             if mds_equal and all(diff is None for diff in [self._on_olt_only_diffs,
                                                            self._on_onu_only_diffs,
@@ -609,14 +619,6 @@
                 # Save the changed data to the MIB.
                 self._database.set(self.device_id, class_id, instance_id, data)
 
-                # Autonomous creation and deletion of managed entities do not
-                # result in an increment of the MIB data sync value. However,
-                # AVC's in response to a change by the Operator do incur an
-                # increment of the MIB Data Sync.  If here during uploading,
-                # we issued a MIB-Reset which may generate AVC.  (TODO: Focus testing during hardening)
-                if self.state == 'uploading':
-                    self.increment_mib_data_sync()
-
             except KeyError:
                 pass            # NOP
 
@@ -711,17 +713,16 @@
                                   status=omci_msg.fields['success_code'],
                                   status_text=self._status_to_text(omci_msg.fields['success_code']),
                                   parameter_error_attributes_mask=omci_msg.fields['parameter_error_attributes_mask'])
-                else:
+
+                elif status != RC.InstanceExists:
                     omci_msg = request.fields['omci_message'].fields
                     class_id = omci_msg['entity_class']
                     entity_id = omci_msg['entity_id']
                     attributes = {k: v for k, v in omci_msg['data'].items()}
 
                     # Save to the database
-                    created = self._database.set(self._device_id, class_id, entity_id, attributes)
-
-                    if created:
-                        self.increment_mib_data_sync()
+                    self._database.set(self._device_id, class_id, entity_id, attributes)
+                    self.increment_mib_data_sync()
 
                     # If the ME contains set-by-create or writeable values that were
                     # not specified in the create command, the ONU will have
@@ -800,10 +801,8 @@
                     entity_id = omci_msg['entity_id']
 
                     # Remove from the database
-                    deleted = self._database.delete(self._device_id, class_id, entity_id)
-
-                    if deleted:
-                        self.increment_mib_data_sync()
+                    self._database.delete(self._device_id, class_id, entity_id)
+                    self.increment_mib_data_sync()
 
             except KeyError as e:
                 pass            # NOP
@@ -822,36 +821,85 @@
         if self._omci_cc_subscriptions[RxEvent.Set]:
             if self.state in ['disabled', 'uploading']:
                 self.log.error('rx-in-invalid-state', state=self.state)
+                return
             try:
                 request = msg[TX_REQUEST_KEY]
                 response = msg[RX_RESPONSE_KEY]
+                tx_omci_msg = request.fields['omci_message'].fields
+                rx_omci_msg = response.fields['omci_message'].fields
 
-                if response.fields['omci_message'].fields['success_code'] != RC.Success:
-                    # TODO: Support offline ONTs in post VOLTHA v1.3.0
-                    omci_msg = response.fields['omci_message']
-                    self.log.warn('set-response-failure',
-                                  class_id=omci_msg.fields['entity_class'],
-                                  instance_id=omci_msg.fields['entity_id'],
-                                  status=omci_msg.fields['success_code'],
-                                  status_text=self._status_to_text(omci_msg.fields['success_code']),
-                                  unsupported_attribute_mask=omci_msg.fields['unsupported_attributes_mask'],
-                                  failed_attribute_mask=omci_msg.fields['failed_attributes_mask'])
+                rx_status = rx_omci_msg['success_code']
+                class_id = rx_omci_msg['entity_class']
+                entity_id = rx_omci_msg['entity_id']
+                attributes = dict()
+
+                tx_mask = tx_omci_msg['attributes_mask']
+                rx_fail_mask = 0
+                if rx_status == RC.AttributeFailure:
+                    rx_fail_mask = rx_omci_msg['unsupported_attributes_mask'] | rx_omci_msg['failed_attributes_mask']
+
+                if rx_status == RC.Success:
+                    attributes = {k: v for k, v in tx_omci_msg['data'].items()}
+
+                elif RC.AttributeFailure and tx_mask != rx_fail_mask:
+                    # Partial success, set only those that were good
+                    entity = self._device.me_map[class_id]
+                    good_mask = tx_mask & ~rx_fail_mask
+                    good_attr_indexes = entity.attribute_indices_from_mask(good_mask)
+                    good_attr_names = {attr.field.name for index, attr in enumerate(entity.attributes)
+                                       if index in good_attr_indexes}
+
+                    attributes = {k: v for k, v in tx_omci_msg['data'].items()
+                                  if k in good_attr_names}
                 else:
-                    omci_msg = request.fields['omci_message'].fields
-                    class_id = omci_msg['entity_class']
-                    entity_id = omci_msg['entity_id']
-                    attributes = {k: v for k, v in omci_msg['data'].items()}
+                    self.log.warn('set-response-failure',
+                                  class_id=rx_omci_msg['entity_class'],
+                                  instance_id=rx_omci_msg['entity_id'],
+                                  status=rx_status,
+                                  status_text=self._status_to_text(rx_status),
+                                  unsupported_attribute_mask=rx_omci_msg['unsupported_attributes_mask'],
+                                  failed_attribute_mask=rx_omci_msg['failed_attributes_mask'])
 
-                    # Save to the database (Do not save 'sets' of the mib-data-sync however)
-                    if class_id != OntData.class_id:
-                        modified = self._database.set(self._device_id, class_id, entity_id, attributes)
-                        if modified:
-                            self.increment_mib_data_sync()
+                # Save to the database. A set of MDS in the OntData class results in
+                # an increment. However, we do not save that within the class/entity
+                # portion of the database.
+                if class_id == OntData.class_id and len(attributes) > 0:
+                    self.increment_mib_data_sync()
+
+                elif len(attributes) > 0:
+                    self._database.set(self._device_id, class_id, entity_id, attributes)
+                    self.increment_mib_data_sync()
 
             except KeyError as _e:
                 pass            # NOP
             except Exception as e:
                 self.log.exception('set', e=e)
+
+    def on_software_event(self, _topic, msg):
+        """
+        Process a Software Start, End, Activate, and Commit
+
+        :param _topic: (str) OMCI-RX topic
+        :param msg: (dict) Dictionary with 'rx-response' and 'tx-request' (if any)
+        """
+        self.log.debug('on-software-event', state=self.state)
+
+        # All events for software run this method, checking for one is good enough
+        if self._omci_cc_subscriptions[RxEvent.Start_Software_Download]:
+            if self.state in ['disabled', 'uploading']:
+                self.log.error('rx-in-invalid-state', state=self.state)
+                return
+            try:
+                # Note: all download responses we subscribe to have a 'result' field
+                response = msg[RX_RESPONSE_KEY]
+                if response.fields['omci_message'].fields['success_code'] == RC.Success:
+                    self.increment_mib_data_sync()
+
+            except KeyError as _e:
+                pass            # NOP
+            except Exception as e:
+                self.log.exception('set', e=e)
+
     def on_capabilities_event(self, _topic, msg):
         """
         Process a OMCI capabilties event
diff --git a/voltha/extensions/omci/tasks/mib_reconcile_task.py b/voltha/extensions/omci/tasks/mib_reconcile_task.py
index 38e29dc..37a05ee 100644
--- a/voltha/extensions/omci/tasks/mib_reconcile_task.py
+++ b/voltha/extensions/omci/tasks/mib_reconcile_task.py
@@ -615,6 +615,17 @@
         returnValue((successes, failures))
 
     @inlineCallbacks
+    def _get_current_mds(self):
+        self.strobe_watchdog()
+        results = yield self._device.omci_cc.send(OntDataFrame().get())
+
+        omci_msg = results.fields['omci_message'].fields
+        status = omci_msg['success_code']
+        mds = (omci_msg['data']['mib_data_sync'] >> 8) & 0xFF \
+            if status == 0 and 'data' in omci_msg and 'mib_data_sync' in omci_msg['data'] else -1
+        returnValue(mds)
+
+    @inlineCallbacks
     def update_mib_data_sync(self):
         """
         As the final step of MIB resynchronization, the OLT sets the MIB data sync
@@ -624,21 +635,27 @@
 
         :return: (int, int) success, failure counts
         """
-        # Get MDS to set, do not user zero
-
+        # Get MDS to set
+        self._sync_sm.increment_mib_data_sync()
         new_mds_value = self._sync_sm.mib_data_sync
-        if new_mds_value == 0:
-            self._sync_sm.increment_mib_data_sync()
-            new_mds_value = self._sync_sm.mib_data_sync
 
         # Update it.  The set response will be sent on the OMCI-CC pub/sub bus
         # and the MIB Synchronizer will update this MDS value in the database
         # if successful.
         try:
+            # previous_mds = yield self._get_current_mds()
+
             frame = OntDataFrame(mib_data_sync=new_mds_value).set()
 
             results = yield self._device.omci_cc.send(frame)
             self.check_status_and_state(results, 'ont-data-mbs-update')
+
+            #########################################
+            # Debug.  Verify new MDS value was received. Should be 1 greater
+            #         than what was sent
+            # new_mds = yield self._get_current_mds()
+            # self.log.info('mds-update', previous=previous_mds, new=new_mds_value, now=new_mds)
+            # Done
             returnValue((1, 0))
 
         except TimeoutError as e:
diff --git a/voltha/extensions/omci/tasks/mib_resync_task.py b/voltha/extensions/omci/tasks/mib_resync_task.py
index ef9c531..b7f3dec 100644
--- a/voltha/extensions/omci/tasks/mib_resync_task.py
+++ b/voltha/extensions/omci/tasks/mib_resync_task.py
@@ -18,12 +18,15 @@
 from twisted.internet import reactor
 from common.utils.asleep import asleep
 from voltha.extensions.omci.database.mib_db_dict import *
-from voltha.extensions.omci.omci_entities import OntData
+from voltha.extensions.omci.omci_entities import OntData, Omci
 from voltha.extensions.omci.omci_defs import AttributeAccess, EntityOperations
+from voltha.extensions.omci.omci_fields import OmciTableField
+from voltha.extensions.omci.omci_me import OntDataFrame
 
 AA = AttributeAccess
 OP = EntityOperations
 
+
 class MibCopyException(Exception):
     pass
 
@@ -230,6 +233,13 @@
             if number_of_commands is None or number_of_commands <= 0:
                 raise ValueError('Number of commands was {}'.format(number_of_commands))
 
+            # Get the current MIB-DATA-SYNC on the ONU
+            self.strobe_watchdog()
+            results = yield self._device.omci_cc.send(OntDataFrame().get())
+            omci_msg = results.fields['omci_message'].fields
+            mds = (omci_msg['data']['mib_data_sync'] >> 8) & 0xFF
+            self._db_active.save_mib_data_sync(self.device_id, mds)
+
             returnValue(number_of_commands)
 
         except TimeoutError as e:
@@ -258,7 +268,7 @@
                     # the device level and do not want it showing up during a re-sync
                     # during data comparison
                     from binascii import hexlify
-                    if class_id == OntData.class_id:
+                    if class_id in (OntData.class_id, Omci.class_id):
                         break
 
                     # The T&W ONU reports an ME with class ID 0 but only on audit. Perhaps others do as well.
@@ -395,13 +405,16 @@
             olt_cls = omci_copy[cls_id]
             onu_cls = onu_copy[cls_id]
 
-            # Weed out read-only attributes. Attributes on onu may be read-only. These
-            # will only show up it the OpenOMCI (OLT-side) database if it changed and
-            # an AVC Notification was sourced by the ONU
+            # Weed out read-only and table attributes. Attributes on onu may be read-only.
+            # These will only show up it the OpenOMCI (OLT-side) database if it changed
+            # and an AVC Notification was sourced by the ONU
             # TODO: These class IDs could be calculated once at ONU startup (at device add)
             if cls_id in me_map:
                 ro_attrs = {attr.field.name for attr in me_map[cls_id].attributes
                             if attr.access == ro_set}
+                table_attrs = {attr.field.name for attr in me_map[cls_id].attributes
+                               if isinstance(attr.field, OmciTableField)}
+
             else:
                 # Here if partially defined ME (not defined in ME Map)
                 from voltha.extensions.omci.omci_cc import UNKNOWN_CLASS_ATTRIBUTE_KEY
@@ -416,11 +429,11 @@
                 onu_attributes = {k for k in onu_cls[inst_id][ATTRIBUTES_KEY].iterkeys()}
 
                 # Get attributes that exist in one database, but not the other
-                sym_diffs = (omci_attributes ^ onu_attributes) - ro_attrs
+                sym_diffs = (omci_attributes ^ onu_attributes) - ro_attrs - table_attrs
                 results.extend([(cls_id, inst_id, attr) for attr in sym_diffs])
 
                 # Get common attributes with different values
-                common_attributes = (omci_attributes & onu_attributes) - ro_attrs
+                common_attributes = (omci_attributes & onu_attributes) - ro_attrs - table_attrs
                 results.extend([(cls_id, inst_id, attr) for attr in common_attributes
                                if olt_cls[inst_id][ATTRIBUTES_KEY][attr] !=
                                 onu_cls[inst_id][ATTRIBUTES_KEY][attr]])
diff --git a/voltha/extensions/omci/tasks/omci_get_request.py b/voltha/extensions/omci/tasks/omci_get_request.py
index 3bcc052..602fe2b 100644
--- a/voltha/extensions/omci/tasks/omci_get_request.py
+++ b/voltha/extensions/omci/tasks/omci_get_request.py
@@ -150,6 +150,28 @@
         attr_def = self._entity_class.attributes[index]
         return isinstance(attr_def.field, OmciTableField)
 
+    def select_first_attributes(self):
+        """
+        For the requested attributes, get as many as possible in the first requst
+        :return: (set) attributes that will fit in one frame
+        """
+        octets_available = 25   # Max GET Response baseline payload size
+        first_attributes = set()
+
+        for attr in self._attributes:
+            index = self._entity_class.attribute_name_to_index_map[attr]
+            attr_def = self._entity_class.attributes[index]
+
+            if isinstance(attr_def.field, OmciTableField):
+                continue   # No table attributes
+
+            size = attr_def.field.sz
+            if size <= octets_available:
+                first_attributes.add(attr)
+                octets_available -= size
+
+        return first_attributes
+
     @inlineCallbacks
     def perform_get_omci(self):
         """
@@ -159,8 +181,7 @@
                       entity_id=self._entity_id, attributes=self._attributes)
         try:
             # If one or more attributes is a table attribute, get it separately
-
-            first_attributes = {attr for attr in self._attributes if not self.is_table_attr(attr)}
+            first_attributes = self.select_first_attributes()
             table_attributes = {attr for attr in self._attributes if self.is_table_attr(attr)}
 
             if len(first_attributes):
@@ -188,10 +209,14 @@
                     results_omci = results.fields['omci_message'].fields
 
                     # Were all attributes fetched?
-                    missing_attr = frame.fields['omci_message'].fields['attributes_mask'] ^ \
-                        results_omci['attributes_mask']
+                    requested_attr = frame.fields['omci_message'].fields['attributes_mask']
+                    retrieved_attr = results_omci['attributes_mask']
+                    unsupported_attr = results_omci.get('unsupported_attributes_mask', 0) or 0
+                    failed_attr = results_omci.get('failed_attributes_mask', 0) or 0
+                    not_avail_attr = unsupported_attr | failed_attr
+                    missing_attr = requested_attr & ~(retrieved_attr | not_avail_attr)
 
-                    if missing_attr > 0 or len(table_attributes) > 0:
+                    if missing_attr != 0 or len(table_attributes) > 0:
                         self.log.info('perform-get-missing', num_missing=missing_attr,
                                       table_attr=table_attributes)
                         self.strobe_watchdog()
@@ -267,7 +292,10 @@
                     status = get_omci['success_code']
 
                     if status == RC.AttributeFailure.value:
-                        # TODO: update failed & unknown attributes set
+                        unsupported_attr = get_omci.get('unsupported_attributes_mask', 0) or 0
+                        failed_attr = get_omci.get('failed_attributes_mask', 0) or 0
+                        results_omci['unsupported_attributes_mask'] |= unsupported_attr
+                        results_omci['failed_attributes_mask'] |= failed_attr
                         continue
 
                     elif status != RC.Success.value:
@@ -293,8 +321,7 @@
         # Now any table attributes
         if len(table_attributes):
             self.strobe_watchdog()
-            self._local_deferred = reactor.callLater(0,
-                                                     self.process_get_table,
+            self._local_deferred = reactor.callLater(0, self.process_get_table,
                                                      table_attributes)
             returnValue(self._local_deferred)
 
@@ -335,8 +362,10 @@
                 if omci_fields['success_code'] == RC.AttributeFailure.value:
                     # Copy over any failed or unsupported attribute masks for final result
                     results_fields = results_omci.fields['omci_message'].fields
-                    results_fields['unsupported_attributes_mask'] |= omci_fields['unsupported_attributes_mask']
-                    results_fields['failed_attributes_mask'] |= omci_fields['failed_attributes_mask']
+                    unsupported_attr = results_omci.get('unsupported_attributes_mask', 0) or 0
+                    failed_attr = results_omci.get('failed_attributes_mask', 0) or 0
+                    results_fields['unsupported_attributes_mask'] |= unsupported_attr
+                    results_fields['failed_attributes_mask'] |= failed_attr
 
                 if omci_fields['success_code'] != RC.Success.value:
                     raise GetException('Get table attribute failed with status code: {}'.