RUNTIME: Variable forcing now uses limited list and buffer instead of systematical instance tree traversal and in-tree "fvalue" to keep track of forced value for pointed variables (external, located). Pointer swapping is performed when forcing externals and located, with backup being restored when forcing is reset. Retain still uses tree traversal. RuntimeLists
authorEdouard Tisserant
Thu, 09 Dec 2021 10:21:45 +0100
branchRuntimeLists
changeset 3395 93ad018fb602
parent 3394 9ea29ac18837
child 3396 8c8cb5c9ff38
RUNTIME: Variable forcing now uses limited list and buffer instead of systematical instance tree traversal and in-tree "fvalue" to keep track of forced value for pointed variables (external, located). Pointer swapping is performed when forcing externals and located, with backup being restored when forcing is reset. Retain still uses tree traversal.
ProjectController.py
runtime/PLCObject.py
runtime/typemapping.py
targets/plc_debug.c
targets/var_access.c
--- a/ProjectController.py	Wed Dec 01 09:54:02 2021 +0100
+++ b/ProjectController.py	Thu Dec 09 10:21:45 2021 +0100
@@ -276,6 +276,7 @@
         # copy StatusMethods so that it can be later customized
         self.StatusMethods = [dic.copy() for dic in self.StatusMethods]
         self.DebugToken = None
+        self.LastComplainDebugToken = None
         self.debug_status = PlcStatus.Stopped
 
         self.IECcodeDigest = None
@@ -1052,9 +1053,7 @@
 
         # prepare debug code
         variable_decl_array = []
-        bofs = 0
         for v in self._DbgVariablesList:
-            sz = DebugTypesSize.get(v["type"], 0)
             variable_decl_array += [
                 "{&(%(C_path)s), " % v +
                 {
@@ -1065,9 +1064,7 @@
                     "VAR": "%(type)s_ENUM"
                 }[v["vartype"]] % v +
                 "}"]
-            bofs += sz
         debug_code = targets.GetCode("plc_debug.c") % {
-            "buffer_size": bofs,
             "programs_declarations": "\n".join(["extern %(type)s %(C_path)s;" %
                                                 p for p in self._ProgramList]),
             "extern_variables_declarations": "\n".join([
@@ -1555,6 +1552,14 @@
                                     else:
                                         values_buffer.append((value, forced))
                             self.DebugTicks.append(debug_tick)
+                        else:
+                            # complain if trace is incomplete, but only once per debug session
+                            if self.LastComplainDebugToken != self.DebugToken :
+                                self.logger.write_warning(
+                                    _("Debug: target couldn't trace all requested variables.\n"))
+                                self.LastComplainDebugToken = self.DebugToken
+
+
 
         buffers, self.DebugValuesBuffers = (self.DebugValuesBuffers,
                                             [list() for dummy in xrange(len(self.TracedIECPath))])
@@ -1563,6 +1568,15 @@
 
         return debug_status, ticks, buffers
 
+    RegisterDebugVariableErrorCodes = { 
+        # TRACE_LIST_OVERFLOW
+        1 : _("Debug: Too many variables traced. Max 1024.\n"),
+        # FORCE_LIST_OVERFLOW
+        2 : _("Debug: Too many variables forced. Max 256.\n"),
+        # FORCE_BUFFER_OVERFLOW
+        3 : _("Debug: Cumulated forced variables size too large. Max 1KB.\n")
+    }
+
     def RegisterDebugVarToConnector(self):
         Idxs = []
         self.TracedIECPath = []
@@ -1596,7 +1610,14 @@
                 IdxsT = zip(*Idxs)
                 self.TracedIECPath = IdxsT[3]
                 self.TracedIECTypes = IdxsT[1]
-                self.DebugToken = self._connector.SetTraceVariablesList(zip(*IdxsT[0:3]))
+                res = self._connector.SetTraceVariablesList(zip(*IdxsT[0:3]))
+                if res is not None and res > 0:
+                    self.DebugToken = res
+                else:
+                    self.DebugToken = None
+                    self.logger.write_warning(
+                        self.RegisterDebugVariableErrorCodes.get(
+                            -res, _("Debug: Unknown error")))
             else:
                 self.TracedIECPath = []
                 self._connector.SetTraceVariablesList([])
--- a/runtime/PLCObject.py	Wed Dec 01 09:54:02 2021 +0100
+++ b/runtime/PLCObject.py	Thu Dec 09 10:21:45 2021 +0100
@@ -223,7 +223,7 @@
             self._ResetDebugVariables.restype = None
 
             self._RegisterDebugVariable = self.PLClibraryHandle.RegisterDebugVariable
-            self._RegisterDebugVariable.restype = None
+            self._RegisterDebugVariable.restype = ctypes.c_int
             self._RegisterDebugVariable.argtypes = [ctypes.c_int, ctypes.c_void_p]
 
             self._FreeDebugData = self.PLClibraryHandle.FreeDebugData
@@ -294,7 +294,7 @@
         self._startPLC = lambda x, y: None
         self._stopPLC = lambda: None
         self._ResetDebugVariables = lambda: None
-        self._RegisterDebugVariable = lambda x, y: None
+        self._RegisterDebugVariable = lambda x, y: 0
         self._IterDebugData = lambda x, y: None
         self._FreeDebugData = lambda: None
         self._GetDebugData = lambda: -1
@@ -720,7 +720,11 @@
                             TypeTranslator.get(iectype,
                                                (None, None, None))
                         force = ctypes.byref(pack_func(c_type, force))
-                    self._RegisterDebugVariable(idx, force)
+                    res = self._RegisterDebugVariable(idx, force)
+                    if res != 0:
+                        self._resumeDebug()
+                        self._suspendDebug(True)
+                        return -res
                 self._TracesSwap()
                 self._resumeDebug()
                 return self.DebugToken
--- a/runtime/typemapping.py	Wed Dec 01 09:54:02 2021 +0100
+++ b/runtime/typemapping.py	Thu Dec 09 10:21:45 2021 +0100
@@ -85,11 +85,22 @@
     for iectype in indexes:
         c_type, unpack_func, _pack_func = \
             TypeTranslator.get(iectype, (None, None, None))
-        if c_type is not None and buffoffset < buffsize:
-            cursor = c_void_p(buffptr + buffoffset)
+
+        cursor = c_void_p(buffptr + buffoffset)
+        if iectype == "STRING":
+            # strlen is stored in c_uint8 and sizeof(c_uint8) is 1
+            # first check we can read size
+            if (buffoffset + 1) <= buffsize:
+                size = 1 + cast(cursor,POINTER(c_type)).contents.len
+            else:
+                return None
+        else:
+            size = sizeof(c_type)
+
+        if c_type is not None and (buffoffset + size) <= buffsize:
             value = unpack_func(cast(cursor,
                                      POINTER(c_type)).contents)
-            buffoffset += sizeof(c_type) if iectype != "STRING" else len(value)+1
+            buffoffset += size
             res.append(value)
         else:
             return None
--- a/targets/plc_debug.c	Wed Dec 01 09:54:02 2021 +0100
+++ b/targets/plc_debug.c	Thu Dec 09 10:21:45 2021 +0100
@@ -25,17 +25,19 @@
 #include <string.h>
 #include <stdio.h>
 
+typedef unsigned int dbgvardsc_index_t;
+typedef unsigned short trace_buf_offset_t;
+
+#define BUFFER_EMPTY 0
+#define BUFFER_FULL 1
+
 #ifndef TARGET_ONLINE_DEBUG_DISABLE
+
 #define TRACE_BUFFER_SIZE 4096
 #define TRACE_LIST_SIZE 1024
 
 /* Atomically accessed variable for buffer state */
-#define BUFFER_FREE 0
-#define BUFFER_BUSY 1
-static long trace_buffer_state = BUFFER_FREE;
-
-typedef unsigned int dbgvardsc_index_t;
-typedef unsigned short trace_buf_offset_t;
+static long trace_buffer_state = BUFFER_EMPTY;
 
 typedef struct trace_item_s {
     dbgvardsc_index_t dbgvardsc_index;
@@ -44,13 +46,36 @@
 trace_item_t trace_list[TRACE_LIST_SIZE];
 char trace_buffer[TRACE_BUFFER_SIZE];
 
-/* Buffer's cursor*/
+/* Trace's cursor*/
 static trace_item_t *trace_list_collect_cursor = trace_list;
 static trace_item_t *trace_list_addvar_cursor = trace_list;
 static const trace_item_t *trace_list_end = 
     &trace_list[TRACE_LIST_SIZE-1];
 static char *trace_buffer_cursor = trace_buffer;
 static const char *trace_buffer_end = trace_buffer + TRACE_BUFFER_SIZE;
+
+
+
+#define FORCE_BUFFER_SIZE 1024
+#define FORCE_LIST_SIZE 256
+
+typedef struct force_item_s {
+    dbgvardsc_index_t dbgvardsc_index;
+    void *value_pointer_backup;
+} force_item_t;
+
+force_item_t force_list[FORCE_LIST_SIZE];
+char force_buffer[FORCE_BUFFER_SIZE];
+
+/* Force's cursor*/
+static force_item_t *force_list_apply_cursor = force_list;
+static force_item_t *force_list_addvar_cursor = force_list;
+static const force_item_t *force_list_end = 
+    &force_list[FORCE_LIST_SIZE-1];
+static char *force_buffer_cursor = force_buffer;
+static const char *force_buffer_end = force_buffer + FORCE_BUFFER_SIZE;
+
+
 #endif
 
 static unsigned int retain_offset = 0;
@@ -92,16 +117,16 @@
 
 void RemindIterator(dbgvardsc_t *dsc)
 {
-    void *real_value_p = NULL;
-    char flags = 0;
-    UnpackVar(dsc, &real_value_p, &flags);
+    void *value_p = NULL;
+    char flags;
+    size_t size;
+    UnpackVar(dsc, &value_p, &flags, &size);
 
     if(flags & __IEC_RETAIN_FLAG){
-        USINT size = __get_type_enum_size(dsc->type);
         /* compute next cursor positon*/
         unsigned int next_retain_offset = retain_offset + size;
         /* if buffer not full */
-        Remind(retain_offset, size, real_value_p);
+        Remind(retain_offset, size, value_p);
         /* increment cursor according size*/
         retain_offset = next_retain_offset;
     }
@@ -117,7 +142,11 @@
     trace_buffer_cursor = trace_buffer;
     trace_list_addvar_cursor = trace_list;
     trace_list_collect_cursor = trace_list;
-    trace_buffer_state = BUFFER_FREE;
+    trace_buffer_state = BUFFER_EMPTY;
+
+    force_buffer_cursor = force_buffer;
+    force_list_addvar_cursor = force_list;
+    force_list_apply_cursor = force_list;
 #endif
 
     retain_offset = 0;
@@ -156,40 +185,21 @@
 
 static inline void BufferIterator(dbgvardsc_t *dsc, int do_debug)
 {
-    void *real_value_p = NULL;
-    void *visible_value_p = NULL;
-    char flags = 0;
-
-    visible_value_p = UnpackVar(dsc, &real_value_p, &flags);
-
-    if(flags & __IEC_RETAIN_FLAG ||
-       ((flags & __IEC_FORCE_FLAG) && (flags & __IEC_OUTPUT_FLAG))){
-        USINT size = __get_type_enum_size(dsc->type);
-
-#ifndef TARGET_ONLINE_DEBUG_DISABLE
-        if((flags & __IEC_FORCE_FLAG) && (flags & __IEC_OUTPUT_FLAG)){
-            if(__Is_a_string(dsc)){
-                /* optimization for strings */
-                size = ((STRING*)visible_value_p)->len + 1;
-            }
-            /* re-force real value of outputs (M and Q)*/
-            memcpy(real_value_p, visible_value_p, size);
-        }
-#endif
-
-        if(flags & __IEC_RETAIN_FLAG){
+    void *value_p = NULL;
+    char flags;
+    size_t size;
+
+    UnpackVar(dsc, &value_p, &flags, &size);
+
+    if(flags & __IEC_RETAIN_FLAG){
+
             /* compute next cursor positon*/
             unsigned int next_retain_offset = retain_offset + size;
             /* if buffer not full */
-            Retain(retain_offset, size, real_value_p);
+            Retain(retain_offset, size, value_p);
             /* increment cursor according size*/
             retain_offset = next_retain_offset;
-        }
-    }
-}
-
-void DebugIterator(dbgvardsc_t *dsc){
-    BufferIterator(dsc, 1);
+    }
 }
 
 void RetainIterator(dbgvardsc_t *dsc){
@@ -202,12 +212,11 @@
 /* GetRetainSizeIterator */
 void GetRetainSizeIterator(dbgvardsc_t *dsc)
 {
-    void *real_value_p = NULL;
-    char flags = 0;
-    UnpackVar(dsc, &real_value_p, &flags);
+    char flags;
+    size_t size;
+    UnpackVar(dsc, NULL, &flags, &size);
 
     if(flags & __IEC_RETAIN_FLAG){
-        USINT size = __get_type_enum_size(dsc->type);
         /* Calc retain buffer size */
         retain_size += size;
     }
@@ -229,6 +238,24 @@
 extern void ValidateRetainBuffer(void);
 extern void InValidateRetainBuffer(void);
 
+#define __ReForceOutput_case_p(TYPENAME)                                                            \
+        case TYPENAME##_P_ENUM :                                                                    \
+        case TYPENAME##_O_ENUM :                                                                    \
+            {                                                                                       \
+                char *next_cursor = force_buffer_cursor + sizeof(TYPENAME);                         \
+                if(next_cursor <= force_buffer_end ){                                               \
+                    /* outputs real value must be systematically forced */                          \
+                    if(vartype == TYPENAME##_O_ENUM)                                                \
+                        /* overwrite value pointed by backup */                                     \
+                        *((TYPENAME *)force_list_apply_cursor->value_pointer_backup) =  \
+                            *((TYPENAME *)force_buffer_cursor);                                     \
+                    /* inc force_buffer cursor */                                                   \
+                    force_buffer_cursor = next_cursor;                                              \
+                }else{                                                                              \
+                    stop = 1;                                                                       \
+                }                                                                                   \
+            }                                                                                       \
+            break;
 void __publish_debug(void)
 {
     retain_offset = 0;
@@ -240,37 +267,51 @@
         /* Lock buffer */
         long latest_state = AtomicCompareExchange(
             &trace_buffer_state,
-            BUFFER_FREE,
-            BUFFER_BUSY);
+            BUFFER_EMPTY,
+            BUFFER_FULL);
             
         /* If buffer was free */
-        if(latest_state == BUFFER_FREE)
+        if(latest_state == BUFFER_EMPTY)
         {
+            int stop = 0;
+            /* Reset force list cursor */
+            force_list_apply_cursor = force_list;
+
+            /* iterate over force list */
+            while(!stop && force_list_apply_cursor < force_list_addvar_cursor){
+                dbgvardsc_t *dsc = &dbgvardsc[
+                    force_list_apply_cursor->dbgvardsc_index];
+                void *varp = dsc->ptr;
+                __IEC_types_enum vartype = dsc->type;
+                switch(vartype){
+                    __ANY(__ReForceOutput_case_p)
+                default:
+                    break;
+                }
+                force_list_apply_cursor++;                                                      \
+            }
+
             /* Reset buffer cursor */
             trace_buffer_cursor = trace_buffer;
             /* Reset trace list cursor */
             trace_list_collect_cursor = trace_list;
-            /* Iterate over all variables to fill debug buffer */
-            __for_each_variable_do(DebugIterator);
+
+            /* iterate over trace list */
             while(trace_list_collect_cursor < trace_list_addvar_cursor){
-                void *real_value_p = NULL;
-                void *visible_value_p = NULL;
-                char flags = 0;
-                USINT size;
+                void *value_p = NULL;
+                size_t size;
                 char* next_cursor;
 
                 dbgvardsc_t *dsc = &dbgvardsc[
                     trace_list_collect_cursor->dbgvardsc_index];
 
-                visible_value_p = UnpackVar(dsc, &real_value_p, &flags);
+                UnpackVar(dsc, &value_p, NULL, &size);
 
                 /* copy visible variable to buffer */;
                 if(__Is_a_string(dsc)){
                     /* optimization for strings */
                     /* assume NULL terminated strings */
-                    size = ((STRING*)visible_value_p)->len + 1;
-                }else{
-                    size = __get_type_enum_size(dsc->type);
+                    size = ((STRING*)value_p)->len + 1;
                 }
 
                 /* compute next cursor positon.*/
@@ -278,7 +319,7 @@
                 /* check for buffer overflow */
                 if(next_cursor < trace_buffer_end)
                     /* copy data to the buffer */
-                    memcpy(trace_buffer_cursor, visible_value_p, size);
+                    memcpy(trace_buffer_cursor, value_p, size);
                 else
                     /* stop looping in case of overflow */
                     break;
@@ -301,69 +342,133 @@
 }
 
 #ifndef TARGET_ONLINE_DEBUG_DISABLE
-#define __RegisterDebugVariable_case_t(TYPENAME) \
-        case TYPENAME##_ENUM :\
-            ((__IEC_##TYPENAME##_t *)varp)->flags |= __IEC_FORCE_FLAG;\
-            ((__IEC_##TYPENAME##_t *)varp)->value = *((TYPENAME *)force);\
-            break;
-#define __RegisterDebugVariable_case_p(TYPENAME)\
-        case TYPENAME##_P_ENUM :\
-            ((__IEC_##TYPENAME##_p *)varp)->flags |= __IEC_FORCE_FLAG;\
-            ((__IEC_##TYPENAME##_p *)varp)->fvalue = *((TYPENAME *)force);\
-            break;\
-        case TYPENAME##_O_ENUM :\
-            ((__IEC_##TYPENAME##_p *)varp)->flags |= __IEC_FORCE_FLAG;\
-            ((__IEC_##TYPENAME##_p *)varp)->fvalue = *((TYPENAME *)force);\
-            *(((__IEC_##TYPENAME##_p *)varp)->value) = *((TYPENAME *)force);\
-            break;
-void RegisterDebugVariable(dbgvardsc_index_t idx, void* force)
-{
+
+#define TRACE_LIST_OVERFLOW    1
+#define FORCE_LIST_OVERFLOW    2
+#define FORCE_BUFFER_OVERFLOW  3
+
+#define __ForceVariable_case_t(TYPENAME)                                                \
+        case TYPENAME##_ENUM :                                                          \
+            /* add to force_list*/                                                      \
+            force_list_addvar_cursor->dbgvardsc_index = idx;                            \
+            ((__IEC_##TYPENAME##_t *)varp)->flags |= __IEC_FORCE_FLAG;                  \
+            ((__IEC_##TYPENAME##_t *)varp)->value = *((TYPENAME *)force);               \
+            break;
+#define __ForceVariable_case_p(TYPENAME)                                                \
+        case TYPENAME##_P_ENUM :                                                        \
+        case TYPENAME##_O_ENUM :                                                        \
+            {                                                                           \
+                char *next_cursor = force_buffer_cursor + sizeof(TYPENAME);             \
+                if(next_cursor <= force_buffer_end ){                                   \
+                    /* add to force_list*/                                              \
+                    force_list_addvar_cursor->dbgvardsc_index = idx;                    \
+                    /* save pointer to backup */                                        \
+                    force_list_addvar_cursor->value_pointer_backup =                    \
+                        ((__IEC_##TYPENAME##_p *)varp)->value;                          \
+                    /* store forced value in force_buffer */                            \
+                    *((TYPENAME *)force_buffer_cursor) = *((TYPENAME *)force);          \
+                    /* replace pointer with pointer to force_buffer */                  \
+                    ((__IEC_##TYPENAME##_p *)varp)->value =                             \
+                        (TYPENAME *)force_buffer_cursor;                                \
+                    /* mark variable as forced */                                       \
+                    ((__IEC_##TYPENAME##_p *)varp)->flags |= __IEC_FORCE_FLAG;          \
+                    /* inc force_buffer cursor */                                       \
+                    force_buffer_cursor = next_cursor;                                  \
+                    /* outputs real value must be systematically forced */              \
+                    if(vartype == TYPENAME##_O_ENUM)                                    \
+                        *(((__IEC_##TYPENAME##_p *)varp)->value) = *((TYPENAME *)force);\
+                } else {                                                                \
+                    error_code = FORCE_BUFFER_OVERFLOW;                                 \
+                    goto error_cleanup;                                                 \
+                }                                                                       \
+            }                                                                           \
+            break;
+
+
+void ResetDebugVariables(void);
+
+int RegisterDebugVariable(dbgvardsc_index_t idx, void* force)
+{
+    int error_code = 0;
     if(idx < sizeof(dbgvardsc)/sizeof(dbgvardsc_t)){
-        /* add to trace_list, inc tracelist_addvar_cursor*/
+        /* add to trace_list, inc trace_list_addvar_cursor*/
         if(trace_list_addvar_cursor <= trace_list_end){
             trace_list_addvar_cursor->dbgvardsc_index = idx;
             trace_list_addvar_cursor++;
+        } else {
+            error_code = TRACE_LIST_OVERFLOW;
+            goto error_cleanup;
         }
         if(force){
-            dbgvardsc_t *dsc = &dbgvardsc[idx];
-            void *varp = dsc->ptr;
-            switch(dsc->type){
-                __ANY(__RegisterDebugVariable_case_t)
-                __ANY(__RegisterDebugVariable_case_p)
-            default:
-                break;
+            if(force_list_addvar_cursor <= force_list_end){
+                dbgvardsc_t *dsc = &dbgvardsc[idx];
+                void *varp = dsc->ptr;
+                __IEC_types_enum vartype = dsc->type;
+
+                switch(vartype){
+                    __ANY(__ForceVariable_case_t)
+                    __ANY(__ForceVariable_case_p)
+                default:
+                    break;
+                }
+                /* inc force_list cursor */
+                force_list_addvar_cursor++;
+            } else {
+                error_code = FORCE_LIST_OVERFLOW;
+                goto error_cleanup;
             }
         }
     }
-}
-
-#define __ResetDebugVariablesIterator_case_t(TYPENAME) \
-        case TYPENAME##_ENUM :\
-            ((__IEC_##TYPENAME##_t *)varp)->flags &= ~(__IEC_FORCE_FLAG);\
-            break;
-
-#define __ResetDebugVariablesIterator_case_p(TYPENAME)\
-        case TYPENAME##_P_ENUM :\
-        case TYPENAME##_O_ENUM :\
-            ((__IEC_##TYPENAME##_p *)varp)->flags &= ~(__IEC_FORCE_FLAG);\
-            break;
-
-void ResetDebugVariablesIterator(dbgvardsc_t *dsc)
-{
-    /* force debug flag to 0*/
-    void *varp = dsc->ptr;
-    switch(dsc->type){
-        __ANY(__ResetDebugVariablesIterator_case_t)
-        __ANY(__ResetDebugVariablesIterator_case_p)
-    default:
-        break;
-    }
-}
+    return 0;
+
+error_cleanup:
+    ResetDebugVariables();
+    trace_buffer_state = BUFFER_EMPTY;
+    return error_code;
+    
+}
+
+#define ResetForcedVariable_case_t(TYPENAME)                                            \
+        case TYPENAME##_ENUM :                                                          \
+            ((__IEC_##TYPENAME##_t *)varp)->flags &= ~__IEC_FORCE_FLAG;                 \
+            /* for local variable we don't restore original value */                    \
+            /* that can be added if needed, but it was like that since ever */          \
+            break;
+
+#define ResetForcedVariable_case_p(TYPENAME)                                            \
+        case TYPENAME##_P_ENUM :                                                        \
+        case TYPENAME##_O_ENUM :                                                        \
+            ((__IEC_##TYPENAME##_p *)varp)->flags &= ~__IEC_FORCE_FLAG;                 \
+            /* restore backup to pointer */                                             \
+            ((__IEC_##TYPENAME##_p *)varp)->value =                                     \
+                force_list_apply_cursor->value_pointer_backup;                          \
+            break;
 
 void ResetDebugVariables(void)
 {
+    /* Reset trace list */
     trace_list_addvar_cursor = trace_list;
-    __for_each_variable_do(ResetDebugVariablesIterator);
+
+    force_list_apply_cursor = force_list;
+    /* Restore forced variables */
+    while(force_list_apply_cursor < force_list_addvar_cursor){
+        dbgvardsc_t *dsc = &dbgvardsc[
+            force_list_apply_cursor->dbgvardsc_index];
+        void *varp = dsc->ptr;
+        switch(dsc->type){
+            __ANY(ResetForcedVariable_case_t)
+            __ANY(ResetForcedVariable_case_p)
+        default:
+            break;
+        }
+        /* inc force_list cursor */
+        force_list_apply_cursor++;
+    } /* else TODO: warn user about failure to force */ 
+
+    /* Reset force list */
+    force_list_addvar_cursor = force_list;
+    /* Reset force buffer */
+    force_buffer_cursor = force_buffer;
 }
 
 void FreeDebugData(void)
@@ -371,8 +476,8 @@
     /* atomically mark buffer as free */
     AtomicCompareExchange(
         &trace_buffer_state,
-        BUFFER_BUSY,
-        BUFFER_FREE);
+        BUFFER_FULL,
+        BUFFER_EMPTY);
 }
 int WaitDebugData(unsigned long *tick);
 /* Wait until debug data ready and return pointer to it */
--- a/targets/var_access.c	Wed Dec 01 09:54:02 2021 +0100
+++ b/targets/var_access.c	Thu Dec 09 10:21:45 2021 +0100
@@ -1,37 +1,33 @@
 
-#define __Unpack_case_t(TYPENAME) \
-        case TYPENAME##_ENUM :\
-            *flags = ((__IEC_##TYPENAME##_t *)varp)->flags;\
-            forced_value_p = *real_value_p = &((__IEC_##TYPENAME##_t *)varp)->value;\
+#define __Unpack_case_t(TYPENAME)                                           \
+        case TYPENAME##_ENUM :                                              \
+            if(flags) *flags = ((__IEC_##TYPENAME##_t *)varp)->flags;       \
+            if(value_p) *value_p = &((__IEC_##TYPENAME##_t *)varp)->value;  \
+		    if(size) *size = sizeof(TYPENAME);                              \
             break;
 
-#define __Unpack_case_p(TYPENAME)\
-        case TYPENAME##_O_ENUM :\
-            *flags = __IEC_OUTPUT_FLAG;\
-        case TYPENAME##_P_ENUM :\
-            *flags |= ((__IEC_##TYPENAME##_p *)varp)->flags;\
-            *real_value_p = ((__IEC_##TYPENAME##_p *)varp)->value;\
-            forced_value_p = &((__IEC_##TYPENAME##_p *)varp)->fvalue;\
+#define __Unpack_case_p(TYPENAME)                                           \
+        case TYPENAME##_O_ENUM :                                            \
+        case TYPENAME##_P_ENUM :                                            \
+            if(flags) *flags = ((__IEC_##TYPENAME##_p *)varp)->flags;       \
+            if(value_p) *value_p = ((__IEC_##TYPENAME##_p *)varp)->value;   \
+		    if(size) *size = sizeof(TYPENAME);                              \
             break;
 
 #define __Is_a_string(dsc) (dsc->type == STRING_ENUM)   ||\
                            (dsc->type == STRING_P_ENUM) ||\
                            (dsc->type == STRING_O_ENUM)
 
-static void* UnpackVar(__Unpack_desc_type *dsc, void **real_value_p, char *flags)
+static int UnpackVar(__Unpack_desc_type *dsc, void **value_p, char *flags, size_t *size)
 {
     void *varp = dsc->ptr;
-    void *forced_value_p = NULL;
-    *flags = 0;
     /* find data to copy*/
     switch(dsc->type){
         __ANY(__Unpack_case_t)
         __ANY(__Unpack_case_p)
     default:
-        break;
+        return 0; /* should never happen */
     }
-    if (*flags & __IEC_FORCE_FLAG)
-        return forced_value_p;
-    return *real_value_p;
+    return 1;
 }