masc2008 commented on code in PR #3459:
URL: https://github.com/apache/nuttx-apps/pull/3459#discussion_r3105057188


##########
modbus/nxmodbus/core/nxmb_client.c:
##########
@@ -0,0 +1,716 @@
+/****************************************************************************
+ * apps/modbus/nxmodbus/core/nxmb_client.c
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * 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 <errno.h>
+#include <stdbool.h>
+#include <stdint.h>
+#include <stdlib.h>
+#include <string.h>
+
+#include <nxmodbus/nxmb_client.h>
+#include <nxmodbus/nxmodbus.h>
+
+#include "nxmb_internal.h"
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+struct nxmb_client_state_s
+{
+  uint32_t timeout_ms;
+};
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+static int nxmb_client_tx_wait_rx(nxmb_handle_t ctx,
+                                  uint8_t expected_uid, uint8_t expected_fc);
+static int nxmb_client_validate_response(nxmb_handle_t ctx,
+                                         uint8_t expected_uid,
+                                         uint8_t expected_fc);
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: nxmb_exception_to_errno
+ ****************************************************************************/
+
+static int nxmb_exception_to_errno(uint8_t exception)
+{
+  switch (exception)
+    {
+      case NXMB_EX_ILLEGAL_FUNCTION:
+        return -ENXIO;
+
+      case NXMB_EX_ILLEGAL_DATA_ADDRESS:
+        return -EFAULT;
+
+      case NXMB_EX_ILLEGAL_DATA_VALUE:
+        return -EINVAL;
+
+      case NXMB_EX_DEVICE_FAILURE:
+        return -EIO;
+
+      case NXMB_EX_ACKNOWLEDGE:
+        return -EBUSY;
+
+      case NXMB_EX_DEVICE_BUSY:
+        return -EBUSY;
+
+      default:
+        return -EPROTO;
+    }
+}
+
+/****************************************************************************
+ * Name: nxmb_client_validate_response
+ ****************************************************************************/
+
+static int nxmb_client_validate_response(nxmb_handle_t ctx,
+                                         uint8_t expected_uid,
+                                         uint8_t expected_fc)
+{
+  uint8_t rx_uid;
+  uint8_t rx_fc;
+
+  if (ctx->adu.length < 2)
+    {
+      return -EPROTO;
+    }
+
+  rx_uid = ctx->adu.unit_id;
+  rx_fc  = ctx->adu.fc;
+
+  if (rx_uid != expected_uid)
+    {
+      return -EPROTO;
+    }
+
+  if (rx_fc == (expected_fc | 0x80))
+    {
+      if (ctx->adu.length < 3)
+        {
+          return -EPROTO;
+        }
+
+      return nxmb_exception_to_errno(ctx->adu.data[0]);
+    }
+
+  if (rx_fc != expected_fc)
+    {
+      return -EPROTO;
+    }
+
+  return OK;
+}
+
+/****************************************************************************
+ * Name: nxmb_client_tx_wait_rx
+ ****************************************************************************/
+
+static int nxmb_client_tx_wait_rx(nxmb_handle_t ctx, uint8_t expected_uid,
+                                  uint8_t expected_fc)
+{
+  FAR struct nxmb_client_state_s *state;
+  uint64_t                        deadline;
+  int                             ret;
+
+  if (ctx == NULL || ctx->client_state == NULL)
+    {
+      return -EINVAL;
+    }
+
+  state = (FAR struct nxmb_client_state_s *)ctx->client_state;
+
+  ret = ctx->transport_ops->send(ctx);
+  if (ret < 0)
+    {
+      return ret;
+    }
+
+  /* Broadcast frames do not receive a response */
+
+  if (expected_uid == NXMB_ADDRESS_BROADCAST)
+    {
+      return OK;
+    }
+
+  deadline = nxmb_util_clock_ms() + state->timeout_ms;
+
+  for (; ; )
+    {
+      if (nxmb_util_clock_ms() >= deadline)
+        {
+          return -ETIMEDOUT;
+        }
+
+      ret = ctx->transport_ops->receive(ctx);
+      if (ret > 0)
+        {
+          return nxmb_client_validate_response(ctx, expected_uid,
+                                               expected_fc);
+        }
+      else if (ret < 0 && ret != -EAGAIN)
+        {
+          return ret;
+        }
+    }
+}
+
+/****************************************************************************
+ * Name: nxmb_client_read_bits
+ *
+ * Description:
+ *   Common helper for FC01 (Read Coils) and FC02 (Read Discrete Inputs).
+ *
+ ****************************************************************************/
+
+static int nxmb_client_read_bits(nxmb_handle_t ctx, uint8_t uid, uint8_t fc,
+                                 uint16_t addr, uint16_t count,
+                                 FAR uint8_t *buf)
+{
+  uint16_t nbytes;
+  int      ret;
+
+  nbytes = (count + 7) / 8;
+
+  pthread_mutex_lock(&ctx->lock);
+
+  ctx->adu.unit_id = uid;
+  ctx->adu.fc = fc;
+  nxmb_util_put_u16_be(&ctx->adu.data[0], addr);
+  nxmb_util_put_u16_be(&ctx->adu.data[2], count);
+  ctx->adu.length = 6;
+
+  ret = nxmb_client_tx_wait_rx(ctx, uid, fc);
+  if (ret < 0)
+    {
+      pthread_mutex_unlock(&ctx->lock);
+      return ret;
+    }
+
+  if (ctx->adu.length < (3 + nbytes) || ctx->adu.data[0] != nbytes)
+    {
+      pthread_mutex_unlock(&ctx->lock);
+      return -EPROTO;
+    }
+
+  memcpy(buf, &ctx->adu.data[1], nbytes);
+
+  pthread_mutex_unlock(&ctx->lock);
+
+  return OK;
+}
+
+/****************************************************************************
+ * Name: nxmb_client_read_regs
+ *
+ * Description:
+ *   Common helper for FC03 (Read Holding) and FC04 (Read Input) registers.
+ *
+ ****************************************************************************/
+
+static int nxmb_client_read_regs(nxmb_handle_t ctx, uint8_t uid, uint8_t fc,
+                                 uint16_t addr, uint16_t count,
+                                 FAR uint16_t *buf)
+{
+  uint16_t nbytes;
+  int      ret;
+  int      i;
+
+  nbytes = count * 2;
+
+  pthread_mutex_lock(&ctx->lock);
+
+  ctx->adu.unit_id = uid;
+  ctx->adu.fc = fc;
+  nxmb_util_put_u16_be(&ctx->adu.data[0], addr);
+  nxmb_util_put_u16_be(&ctx->adu.data[2], count);
+  ctx->adu.length = 6;
+
+  ret = nxmb_client_tx_wait_rx(ctx, uid, fc);
+  if (ret < 0)
+    {
+      pthread_mutex_unlock(&ctx->lock);
+      return ret;
+    }
+
+  if (ctx->adu.length < (3 + nbytes) || ctx->adu.data[0] != nbytes)
+    {
+      pthread_mutex_unlock(&ctx->lock);
+      return -EPROTO;
+    }
+
+  for (i = 0; i < count; i++)
+    {
+      buf[i] = nxmb_util_get_u16_be(&ctx->adu.data[1 + i * 2]);
+    }
+
+  pthread_mutex_unlock(&ctx->lock);
+
+  return OK;
+}
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: nxmb_read_coils
+ ****************************************************************************/
+
+int nxmb_read_coils(nxmb_handle_t ctx, uint8_t uid, uint16_t addr,
+                    uint16_t count, FAR uint8_t *buf)
+{
+  DEBUGASSERT(ctx && buf);
+
+  if (uid == NXMB_ADDRESS_BROADCAST || count == 0 || count > 2000)
+    {
+      return -EINVAL;
+    }
+
+  if (!ctx->is_client)
+    {
+      return -ENOTSUP;
+    }
+
+  return nxmb_client_read_bits(ctx, uid, NXMB_FC_READ_COILS, addr,
+                               count, buf);
+}
+
+/****************************************************************************
+ * Name: nxmb_read_discrete
+ ****************************************************************************/
+
+int nxmb_read_discrete(nxmb_handle_t ctx, uint8_t uid, uint16_t addr,
+                       uint16_t count, FAR uint8_t *buf)
+{
+  DEBUGASSERT(ctx && buf);
+
+  if (uid == NXMB_ADDRESS_BROADCAST || count == 0 || count > 2000)
+    {
+      return -EINVAL;
+    }
+
+  if (!ctx->is_client)
+    {
+      return -ENOTSUP;
+    }
+
+  return nxmb_client_read_bits(ctx, uid, NXMB_FC_READ_DISCRETE, addr,
+                               count, buf);
+}
+
+/****************************************************************************
+ * Name: nxmb_read_input
+ ****************************************************************************/
+
+int nxmb_read_input(nxmb_handle_t ctx, uint8_t uid, uint16_t addr,
+                    uint16_t count, FAR uint16_t *buf)
+{
+  DEBUGASSERT(ctx && buf);
+
+  if (uid == NXMB_ADDRESS_BROADCAST || count == 0 || count > 125)
+    {
+      return -EINVAL;
+    }
+
+  if (!ctx->is_client)
+    {
+      return -ENOTSUP;
+    }
+
+  return nxmb_client_read_regs(ctx, uid, NXMB_FC_READ_INPUT, addr,
+                               count, buf);
+}
+
+/****************************************************************************
+ * Name: nxmb_read_holding
+ ****************************************************************************/
+
+int nxmb_read_holding(nxmb_handle_t ctx, uint8_t uid, uint16_t addr,
+                      uint16_t count, FAR uint16_t *buf)
+{
+  DEBUGASSERT(ctx && buf);
+
+  if (uid == NXMB_ADDRESS_BROADCAST || count == 0 || count > 125)
+    {
+      return -EINVAL;
+    }
+
+  if (!ctx->is_client)
+    {
+      return -ENOTSUP;
+    }
+
+  return nxmb_client_read_regs(ctx, uid, NXMB_FC_READ_HOLDING, addr,
+                               count, buf);
+}
+
+/****************************************************************************
+ * Name: nxmb_write_coil
+ ****************************************************************************/
+
+int nxmb_write_coil(nxmb_handle_t ctx, uint8_t uid, uint16_t addr,
+                    bool value)
+{
+  uint16_t coil_value;
+  int      ret;
+
+  DEBUGASSERT(ctx);
+
+  if (!ctx->is_client)
+    {
+      return -ENOTSUP;
+    }
+
+  coil_value = value ? 0xff00 : 0x0000;
+
+  pthread_mutex_lock(&ctx->lock);
+
+  ctx->adu.unit_id = uid;
+  ctx->adu.fc = NXMB_FC_WRITE_COIL;
+  nxmb_util_put_u16_be(&ctx->adu.data[0], addr);
+  nxmb_util_put_u16_be(&ctx->adu.data[2], coil_value);
+  ctx->adu.length = 6;
+
+  ret = nxmb_client_tx_wait_rx(ctx, uid, NXMB_FC_WRITE_COIL);
+  if (ret < 0)
+    {
+      pthread_mutex_unlock(&ctx->lock);
+      return ret;
+    }
+
+  if (uid != NXMB_ADDRESS_BROADCAST && ctx->adu.length < 6)
+    {
+      pthread_mutex_unlock(&ctx->lock);
+      return -EPROTO;
+    }
+
+  pthread_mutex_unlock(&ctx->lock);
+
+  return OK;
+}
+
+/****************************************************************************
+ * Name: nxmb_write_holding
+ ****************************************************************************/
+
+int nxmb_write_holding(nxmb_handle_t ctx, uint8_t uid, uint16_t addr,
+                       uint16_t value)
+{
+  int ret;
+
+  DEBUGASSERT(ctx);
+
+  if (!ctx->is_client)
+    {
+      return -ENOTSUP;
+    }
+
+  pthread_mutex_lock(&ctx->lock);
+
+  ctx->adu.unit_id = uid;
+  ctx->adu.fc = NXMB_FC_WRITE_HOLDING;
+  nxmb_util_put_u16_be(&ctx->adu.data[0], addr);
+  nxmb_util_put_u16_be(&ctx->adu.data[2], value);
+  ctx->adu.length = 6;
+
+  ret = nxmb_client_tx_wait_rx(ctx, uid, NXMB_FC_WRITE_HOLDING);
+  if (ret < 0)
+    {
+      pthread_mutex_unlock(&ctx->lock);
+      return ret;
+    }
+
+  if (uid != NXMB_ADDRESS_BROADCAST && ctx->adu.length < 6)
+    {
+      pthread_mutex_unlock(&ctx->lock);
+      return -EPROTO;
+    }
+
+  pthread_mutex_unlock(&ctx->lock);
+
+  return OK;
+}
+
+/****************************************************************************
+ * Name: nxmb_write_coils
+ ****************************************************************************/
+
+int nxmb_write_coils(nxmb_handle_t ctx, uint8_t uid, uint16_t addr,
+                     uint16_t count, FAR const uint8_t *buf)
+{
+  uint16_t nbytes;
+  int      ret;
+
+  DEBUGASSERT(ctx && buf);
+
+  if (count == 0 || count > 1968)
+    {
+      return -EINVAL;
+    }
+
+  if (!ctx->is_client)
+    {
+      return -ENOTSUP;
+    }
+
+  nbytes = (count + 7) / 8;
+
+  pthread_mutex_lock(&ctx->lock);
+
+  ctx->adu.unit_id = uid;
+  ctx->adu.fc = NXMB_FC_WRITE_COILS;
+  nxmb_util_put_u16_be(&ctx->adu.data[0], addr);
+  nxmb_util_put_u16_be(&ctx->adu.data[2], count);
+  ctx->adu.data[4] = nbytes;
+  memcpy(&ctx->adu.data[5], buf, nbytes);

Review Comment:
   it's better to do a sanity check.
   in adu structure: data[NXMB_ADU_DATA_MAX]; this is an array that size can be 
configured.
   it  can overflow ctx->adu.data[] if "count > NXMB_ADU_DATA_MAX".
   similar check needed nxmb_write_holdings and nxmb_readwrite_holdings.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to