mlyszczek commented on code in PR #1460:
URL: https://github.com/apache/nuttx-apps/pull/1460#discussion_r1054774628


##########
logging/nxscope/nxscope_chan.c:
##########
@@ -0,0 +1,1566 @@
+/****************************************************************************
+ * apps/logging/nxscope/nxscope_chan.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <assert.h>
+#include <debug.h>
+#include <errno.h>
+#include <string.h>
+#include <stdlib.h>
+
+#include <logging/nxscope/nxscope.h>
+
+#include "nxscope_internals.h"
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#ifdef CONFIG_ENDIAN_BIG
+#  error Big endian not tested
+#endif
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+int g_type_size[] =
+{
+  0,                            /* NXSCOPE_TYPE_UNDEF */
+  0,                            /* NXSCOPE_TYPE_NONE */
+  sizeof(uint8_t),              /* NXSCOPE_TYPE_UINT8 */
+  sizeof(int8_t),               /* NXSCOPE_TYPE_INT8 */
+  sizeof(uint16_t),             /* NXSCOPE_TYPE_UINT16 */
+  sizeof(int16_t),              /* NXSCOPE_TYPE_INT16 */
+  sizeof(uint32_t),             /* NXSCOPE_TYPE_UINT32 */
+  sizeof(int32_t),              /* NXSCOPE_TYPE_INT32 */
+  sizeof(uint64_t),             /* NXSCOPE_TYPE_UINT64 */
+  sizeof(int64_t),              /* NXSCOPE_TYPE_INT64 */
+  sizeof(float),                /* NXSCOPE_TYPE_FLOAT */
+  sizeof(double),               /* NXSCOPE_TYPE_DOUBLE */
+  sizeof(ub8_t),                /* NXSCOPE_TYPE_UB8 */
+  sizeof(b8_t),                 /* NXSCOPE_TYPE_B8 */
+  sizeof(ub16_t),               /* NXSCOPE_TYPE_UB16 */
+  sizeof(b16_t),                /* NXSCOPE_TYPE_B16 */
+  sizeof(ub32_t),               /* NXSCOPE_TYPE_UB32 */
+  sizeof(b32_t),                /* NXSCOPE_TYPE_B32 */
+  sizeof(char),                 /* NXSCOPE_TYPE_CHAR */
+#if 0
+  /* Reserved for future use */
+
+  sizeof(wchar_t),              /* NXSCOPE_TYPE_WCHAR */
+#endif
+
+#ifdef CONFIG_LOGGING_NXSCOPE_USERTYPES
+  /* User type always last element and always equal to  1B */
+
+  sizeof(uint8_t),              /* NXSCOPE_TYPE_USER */
+#endif
+};
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: nxscope_stream_overflow
+ *
+ * NOTE: This function assumes that we have exclusive access to the nxscope
+ *       instance
+ *
+ ****************************************************************************/
+
+static void nxscope_stream_overflow(FAR struct nxscope_s *s)
+{
+  DEBUGASSERT(s);
+
+  s->streambuf[s->proto_stream->hdrlen] |= NXSCOPE_STREAM_FLAGS_OVERFLOW;
+}
+
+/****************************************************************************
+ * Name: nxscope_ch_validate
+ ****************************************************************************/
+
+static int nxscope_ch_validate(FAR struct nxscope_s *s, uint8_t ch,
+                               uint8_t type, uint8_t d, uint8_t mlen)
+{
+  union nxscope_chinfo_type_u utype;
+  size_t                      next_i    = 0;
+  int                         ret       = OK;
+  size_t                      type_size = 0;
+
+  DEBUGASSERT(s);
+
+  /* Do nothing if stream not started */
+
+  if (!s->start)
+    {
+      ret = -EAGAIN;
+      goto errout;
+    }
+
+  /* Do nothing if channel not enabled */
+
+  if (s->chinfo[ch].enable != 1)
+    {
+      ret = -EAGAIN;
+      goto errout;
+    }
+
+  /* Some additional checks if debug features enabled */
+
+#ifdef CONFIG_DEBUG_FEATURES
+  /* Validate channel */
+
+  if (ch > s->cmninfo.chmax)
+    {
+      _err("ERROR: invalid channel %d\n", ch);
+      ret = -EINVAL;
+      goto errout;
+    }
+
+  /* Validate channel type */
+
+  if (s->chinfo[ch].type.s.dtype != type)
+    {
+      _err("ERROR: invalid channel type %d != %d\n",
+           s->chinfo[ch].type.u8, type);
+      ret = -EINVAL;
+      goto errout;
+    }
+
+  /* Validate channel vdim */
+
+  if (s->chinfo[ch].vdim != d)
+    {
+      _err("ERROR: invalid channel dim %d\n", d);
+      ret = -EINVAL;
+      goto errout;
+    }
+
+  /* Validate channel metadata size */
+
+  if (s->chinfo[ch].mlen != mlen)
+    {
+      _err("ERROR: invalid channel mlen %d\n", mlen);
+      ret = -EINVAL;
+      goto errout;
+    }
+#endif
+
+#ifdef CONFIG_LOGGING_NXSCOPE_DIVIDER
+  /* Handle sample rate divider */
+
+  s->cntr[ch] += 1;
+  if (s->cntr[ch] % (s->chinfo[ch].div + 1) != 0)
+    {
+      ret = -EAGAIN;
+      goto errout;
+    }
+#endif
+
+  /* Get utype */
+
+  utype.u8 = type;
+
+#ifdef CONFIG_LOGGING_NXSCOPE_CRICHANNELS
+  /* No more checks needed for critical channel */
+
+  if (utype.s.cri)
+    {
+      ret = OK;
+      goto errout;
+    }
+#endif
+
+  /* Check buffer size */
+
+#ifdef CONFIG_LOGGING_NXSCOPE_USERTYPES
+  if (type >= NXSCOPE_TYPE_USER)
+    {
+      type_size = 1;
+    }
+  else
+#endif
+    {
+      type_size = g_type_size[utype.s.dtype];
+    }
+
+  next_i = (s->stream_i + 1 + type_size * d + mlen +
+            s->proto_stream->footlen);
+
+  if (next_i > s->streambuf_len)
+    {
+      _err("ERROR: no space for data %d\n", s->stream_i);
+      nxscope_stream_overflow(s);
+      ret = -ENOBUFS;
+      goto errout;
+    }
+
+errout:
+  return ret;
+}
+
+/****************************************************************************
+ * Name: nxscope_put_vector
+ *
+ * NOTE: This function assumes that we have exclusive access to the nxscope
+ *       stream buffer
+ *
+ * IMPORTANT: Data stored always as little-endian !
+ *
+ ****************************************************************************/
+
+static int nxscope_put_vector(FAR uint8_t *buff, uint8_t type, FAR void *val,
+                              uint8_t d)
+{
+  int i = 0;
+  int j = 0;
+
+  DEBUGASSERT(buff);
+
+  /* Pack data */
+
+  switch (type)
+    {
+      case NXSCOPE_TYPE_NONE:
+        {
+          /* Nothing to do here */
+
+          break;
+        }
+
+      case NXSCOPE_TYPE_UINT8:
+      case NXSCOPE_TYPE_INT8:
+#ifdef CONFIG_LOGGING_NXSCOPE_USERTYPES
+      case NXSCOPE_TYPE_USER:
+#endif
+        {
+          uint8_t u8 = 0;
+
+          for (i = 0; i < d; i++)
+            {
+              DEBUGASSERT(val);
+              u8 = ((FAR uint8_t *)val)[i];
+
+              buff[j++] = u8;
+            }
+
+          break;
+        }
+
+      case NXSCOPE_TYPE_UINT16:
+      case NXSCOPE_TYPE_INT16:
+      case NXSCOPE_TYPE_B8:
+      case NXSCOPE_TYPE_UB8:
+        {
+          uint16_t u16 = 0;
+
+          for (i = 0; i < d; i++)
+            {
+              DEBUGASSERT(val);
+              u16 = ((FAR uint16_t *)val)[i];
+#ifdef CONFIG_ENDIAN_BIG
+              u16 = __swap_uint16(u16);
+#endif
+
+              buff[j++] = ((u16 >> 0) & 0xff);
+              buff[j++] = ((u16 >> 8) & 0xff);
+            }
+
+          break;
+        }
+
+      case NXSCOPE_TYPE_UINT32:
+      case NXSCOPE_TYPE_INT32:
+      case NXSCOPE_TYPE_FLOAT:
+      case NXSCOPE_TYPE_B16:
+      case NXSCOPE_TYPE_UB16:
+        {
+          uint32_t u32 = 0;
+
+          for (i = 0; i < d; i++)
+            {
+              DEBUGASSERT(val);
+              u32 = ((FAR uint32_t *)val)[i];
+#ifdef CONFIG_ENDIAN_BIG
+              u32 = __swap_uint32(u32);
+#endif
+
+              buff[j++] = ((u32 >> 0) & 0xff);
+              buff[j++] = ((u32 >> 8) & 0xff);
+              buff[j++] = ((u32 >> 16) & 0xff);
+              buff[j++] = ((u32 >> 24) & 0xff);
+            }
+
+          break;
+        }
+
+      case NXSCOPE_TYPE_UINT64:
+      case NXSCOPE_TYPE_INT64:
+      case NXSCOPE_TYPE_DOUBLE:
+      case NXSCOPE_TYPE_B32:
+      case NXSCOPE_TYPE_UB32:
+        {
+          for (i = 0; i < d; i++)
+            {
+              uint64_t u64 = 0;
+
+              DEBUGASSERT(val);
+              u64 = ((FAR uint64_t *)val)[i];
+#ifdef CONFIG_ENDIAN_BIG
+              u64 = __swap_uint64(u64);
+#endif
+
+              buff[j++] = ((u64 >> 0) & 0xff);
+              buff[j++] = ((u64 >> 8) & 0xff);
+              buff[j++] = ((u64 >> 16) & 0xff);
+              buff[j++] = ((u64 >> 24) & 0xff);
+              buff[j++] = ((u64 >> 32) & 0xff);
+              buff[j++] = ((u64 >> 40) & 0xff);
+              buff[j++] = ((u64 >> 48) & 0xff);
+              buff[j++] = ((u64 >> 56) & 0xff);
+            }
+
+          break;
+        }
+
+      case NXSCOPE_TYPE_CHAR:
+        {
+          /* Copy only string bytes + '\0' */
+
+          DEBUGASSERT(val);
+
+          strncpy((FAR char *)buff, (FAR const char *)val, d);
+          j += strnlen((FAR char *)buff, d);
+          buff[j++] = '\0';
+
+          break;
+        }
+
+      default:
+        {
+          _err("ERROR: invalid type=%d\n", type);
+          DEBUGASSERT(0);
+        }
+    }
+
+  return j;
+}
+
+/****************************************************************************
+ * Name: nxscope_put_meta
+ *
+ * NOTE: This function assumes that we have exclusive access to the nxscope
+ *       stream buffer
+ *
+ * REVISIT: what about endianness ?
+ *
+ ****************************************************************************/
+
+static int nxscope_put_meta(FAR uint8_t *buff, FAR uint8_t *meta,
+                            uint8_t mlen)
+{
+  int i = 0;
+
+  DEBUGASSERT(buff);
+
+  for (i = 0; i < mlen; i++)
+    {
+      DEBUGASSERT(meta);
+      buff[i] = meta[i];
+    }
+
+  return mlen;
+}
+
+/****************************************************************************
+ * Name: nxscope_put_sample
+ *
+ * NOTE: This function assumes that we have exclusive access to the nxscope
+ *       stream buffer
+ *
+ ****************************************************************************/
+
+static void nxscope_put_sample(FAR uint8_t *buff, FAR size_t *buff_i,
+                               uint8_t type, uint8_t ch, FAR void *val,
+                               uint8_t d, FAR uint8_t *meta, uint8_t mlen)
+{
+  size_t i = 0;
+
+  /* Channel ID */
+
+  buff[(*buff_i)++] = ch;
+
+  /* Vector sample data - always little-endian */
+
+  i = nxscope_put_vector(&buff[*buff_i], type, val, d);
+  *buff_i += i;
+
+  /* Meta data.
+   * REVISIT: what about endianness ?
+   */
+
+  i = nxscope_put_meta(&buff[*buff_i], meta, mlen);
+  *buff_i += i;
+}
+
+/****************************************************************************
+ * Name: nxscope_put_common_m
+ ****************************************************************************/
+
+static int nxscope_put_common_m(FAR struct nxscope_s *s, uint8_t type,
+                                uint8_t ch, FAR void *val, uint8_t d,
+                                FAR uint8_t *meta, uint8_t mlen)
+{
+  FAR uint8_t                 *buff   = NULL;
+  FAR size_t                  *buff_i = NULL;
+  int                          ret    = OK;
+#ifdef CONFIG_LOGGING_NXSCOPE_CRICHANNELS
+  size_t                       tmp    = 0;
+  union nxscope_chinfo_type_u  utype;
+#endif
+
+  DEBUGASSERT(s);
+
+  nxscope_lock(s);
+
+  /* Validate data */
+
+  ret = nxscope_ch_validate(s, ch, type, d, mlen);
+  if (ret != OK)
+    {
+      goto errout;
+    }
+
+  /* Get buffer to send */
+
+#ifdef CONFIG_LOGGING_NXSCOPE_CRICHANNELS
+  utype.u8 = type;
+  if (utype.s.cri)
+    {
+      /* Dedicated critical channels buffer */
+
+      buff   = s->cribuf;
+      buff_i = &tmp;
+    }
+  else
+#endif
+    {
+      /* Common stream buffer */
+
+      buff   = s->streambuf;
+      buff_i = &s->stream_i;
+    }
+
+  /* Put sample on buffer */
+
+  nxscope_put_sample(buff, buff_i, type, ch, val, d, meta, mlen);
+
+#ifdef CONFIG_LOGGING_NXSCOPE_CRICHANNELS
+  if (utype.s.cri)
+    {
+      /* Send data without buffering */
+
+      ret = nxscope_stream_send(s, buff, buff_i);
+      if (ret < 0)
+        {
+          _err("ERROR: nxscope_stream_send failed %d\n", ret);
+          goto errout;
+        }
+    }
+#endif
+
+errout:
+  nxscope_unlock(s);
+
+  return ret;
+}
+
+#ifdef CONFIG_LOGGING_NXSCOPE_CRICHANNELS
+/****************************************************************************
+ * Name: nxscope_cribuf_alloc
+ ****************************************************************************/
+
+static int nxscope_cribuf_alloc(FAR struct nxscope_s *s, uint8_t dtype,
+                                uint8_t vdim, uint8_t mlen)
+{
+  size_t reqlen    = 0;
+  size_t type_size = 0;
+  int    ret       = OK;
+
+  DEBUGASSERT(s);
+
+  /* We use one additiona buffer to handle no-buffered critical channels.
+   * We have to allocate memory to fit at least single frame with
+   * the longest possible critical channel sample data.
+   */
+
+  type_size = g_type_size[dtype];
+  reqlen = (s->proto_stream->hdrlen + 1 + type_size * vdim +
+            mlen + s->proto_stream->footlen);
+
+  /* This is the first time allocation or we need more memory */
+
+  if (reqlen > s->cri_len)
+    {
+      s->cribuf = realloc(s->cribuf, reqlen);
+      if (s->cribuf == NULL)
+        {
+          ret = -errno;
+          _err("ERROR: cribuf realloc failed %d\n", ret);
+          goto errout;
+        }
+
+      s->cri_len = reqlen;
+    }
+
+errout:
+  return ret;
+}
+#endif
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: nxscope_chan_init
+ *
+ * Description:
+ *   Initialize nxscope channel
+ *
+ * Input Parameters:
+ *   s    - a pointer to a nxscope instance
+ *   ch   - a channel id
+ *   name - a channel name
+ *   type - a channel data type (union nxscope_chinfo_type_u)
+ *   vdim - a vector data dimension (vdim=1 for a point)
+ *   mlen - a length of metadata
+ *
+ ****************************************************************************/
+
+int nxscope_chan_init(FAR struct nxscope_s *s, uint8_t ch, FAR char *name,
+                      uint8_t type, uint8_t vdim, uint8_t mlen)
+{
+  union nxscope_chinfo_type_u utype;
+  size_t                      txbuf_len = 0;
+  int                         ret = OK;
+
+  DEBUGASSERT(s);
+  DEBUGASSERT(name);
+
+  if (ch > s->cmninfo.chmax)
+    {
+      _err("ERROR: invalid channel %d\n", ch);
+      ret = -EINVAL;
+      goto errout;
+    }
+
+  nxscope_lock(s);
+
+  utype.u8 = type;
+  if (utype.s.cri)
+#ifdef CONFIG_LOGGING_NXSCOPE_CRICHANNELS
+    {
+      /* Allocate or re-allocate critical channels buffer */
+
+      ret = nxscope_cribuf_alloc(s, utype.s.dtype, vdim, mlen);
+      if (ret < 0)
+        {
+          _err("ERROR: nxscope_cribuf_alloc failed %d\n", ret);
+          ret = -EINVAL;
+          goto errout;
+        }
+    }
+#else
+    {
+      _err("ERROR: cri channels not supported ch=%d\n", ch);
+      ret = -EINVAL;
+      goto errout;
+    }
+#endif
+
+  /* Reallocate TX buffer if needed
+   *
+   * REVISIT:
+   *   In the worst case this give us chmax reallocationsm but we don't know
+   *   how much memory is needed for the channel with the longest channel
+   *   name. Or just allocate memor for CHAN_NAMELEN_MAX ?
+   */
+
+  txbuf_len = (sizeof(struct nxscope_chinfo_s) +
+               strnlen(name, CHAN_NAMELEN_MAX) + 1);
+  ret = nxscope_txbuf_realloc(s, txbuf_len);
+  if (ret < 0)
+    {
+      _err("ERROR: txbuf realloc failed %d\n", ret);
+      goto errout;
+    }
+
+  /* Reset channel data */
+
+  memset(&s->chinfo[ch], 0, sizeof(struct nxscope_chinfo_s));
+
+  /* Copy channel info */
+
+  s->chinfo[ch].type.u8 = type;
+  s->chinfo[ch].vdim    = vdim;
+  s->chinfo[ch].mlen    = mlen;
+  s->chinfo[ch].name    = name;
+
+  nxscope_unlock(s);
+
+errout:
+  return ret;
+}
+
+/****************************************************************************
+ * Name: nxscope_chan_en
+ *
+ * Description:
+ *   Enable/disable a given channel
+ *
+ * Input Parameters:
+ *   s  - a pointer to a nxscope instance
+ *   ch - a channel id
+ *   en - enable/disable
+ *
+ ****************************************************************************/
+
+int nxscope_chan_en(FAR struct nxscope_s *s, uint8_t ch, bool en)
+{
+  int ret = OK;
+
+  DEBUGASSERT(s);
+
+  nxscope_lock(s);
+
+  if (ch > s->cmninfo.chmax)
+    {
+      _err("ERROR: invalid channel %d\n", ch);
+      ret = -EINVAL;
+      goto errout;
+    }
+
+  _info("chan_en=%d %d\n", ch, en);
+
+  /* Set enable flag */
+
+  s->chinfo[ch].enable = en;
+
+errout:
+  nxscope_unlock(s);
+
+  return ret;
+}
+
+#ifdef CONFIG_LOGGING_NXSCOPE_DIVIDER
+/****************************************************************************
+ * Name: nxscope_chan_div
+ *
+ * Description:
+ *   Configure divider for a given channel
+ *
+ * Input Parameters:
+ *   s   - a pointer to a nxscope instance
+ *   ch  - a channel id
+ *   div - divider value - starts from 0
+ *
+ ****************************************************************************/
+
+int nxscope_chan_div(FAR struct nxscope_s *s, uint8_t ch, uint8_t div)
+{
+  int ret = OK;
+
+  DEBUGASSERT(s);
+
+  nxscope_lock(s);
+
+  if (ch > s->cmninfo.chmax)
+    {
+      _err("ERROR: invalid channel %d\n", ch);
+      ret = -EINVAL;
+      goto errout;
+    }
+
+  _info("chan_div=%d %d\n", ch, div);
+
+  /* Set divider */
+
+  s->chinfo[ch].div = div;
+
+errout:
+  nxscope_unlock(s);
+
+  return ret;
+}
+#endif
+
+/****************************************************************************
+ * Name: nxscope_chan_all_en
+ *
+ * Description:
+ *   Enable/disable all channels
+ *
+ * Input Parameters:
+ *   s  - a pointer to a nxscope instance
+ *   en - enable/disable
+ *
+ ****************************************************************************/
+
+int nxscope_chan_all_en(FAR struct nxscope_s *s, bool en)
+{
+  int ret = OK;
+  int i   = 0;
+
+  DEBUGASSERT(s);
+
+  for (i = 0; i < s->cmninfo.chmax; i++)
+    {
+      ret = nxscope_chan_en(s, i, en);
+      if (ret < 0)
+        {
+          goto errout;

Review Comment:
   Are you sure you want to exit early when one channel fails to 
enable/disable? Wouldn't it be better to disable/enable all channels, but 
return error if at least one channel failed? `nxscope_chan_en` returns only one 
error so you could do `ret |= nxscope_chan_en(s, i, en);` and skip the `if (ret 
< 0)`



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscr...@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to