You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by "Otpvondoiats (via GitHub)" <gi...@apache.org> on 2024/09/19 06:20:07 UTC

[PR] sensor:Fixed the problem of user information lag in cross-core communication "stublist". [nuttx]

Otpvondoiats opened a new pull request, #13538:
URL: https://github.com/apache/nuttx/pull/13538

   ## Summary
   - Bug fix. When the remote core publishes a message, the subscribed cores will receive the message. When the subscribed core publishes a new message, it will take away the message published by the remote core, so all stublist information needs to be updated.
   
   ## Impact
   - Sensor cross-core communication.
   
   ## Testing
   ```
   /****************************************************************************
    * apps/examples/hello/hello_main.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 <stdio.h>
   #include <errno.h>
   #include <math.h>
   #include <poll.h>
   #include <string.h>
   #include <stdarg.h>
   #include <stdlib.h>
   #include <unistd.h>
   #include <inttypes.h>
   
   #include <uORB/uORB.h>
   #include <sensor/gps.h>
   #include <uv.h>
   
   /****************************************************************************
    * Public Functions
    ****************************************************************************/
   
   /****************************************************************************
    * hello_main
    ****************************************************************************/
   
    void cp_advertiser(void);
    void cp_subscriber(void);
    void sensor_adv_sub_er(void);
   
   enum FakeCmd {
     FAKE_CONTROL_CMD,
     FAKE_DISTRIBUTE_CMD,
   };
   
   enum FakeAttribute {
     HELLO_QUEUE_SIZE = 5,
     HELLO_SLEEP_DURA = 20,
   };
   
   int main(int argc, FAR char *argv[])
   {
     printf("Hello, World!!\n");
     if (argc == 1) {
       cp_advertiser();
     } else if (argc == 2) {
       cp_subscriber();
     }else {
       sensor_adv_sub_er();
     }
     return 0;
   }
   
    void cp_advertiser(void) {
     int instance = 0;
     int adv_fd = orb_advertise_multi_queue(ORB_ID(sensor_gps), NULL, &instance, HELLO_QUEUE_SIZE);
     syslog(3, "DEBUG_IN_HELLO, cp adv:%d\n", adv_fd);
   
     while(true) {
       sleep(HELLO_SLEEP_DURA);
       struct sensor_gps gps = {};
       gps.timestamp = (uint64_t)time(NULL);
       gps.time_utc = FAKE_CONTROL_CMD;
       int ret = orb_publish(ORB_ID(sensor_gps), adv_fd, &gps);
       syslog(3, "DEBUG_IN_HELLO, cp send ctl:%d,%" PRIu64 ",%" PRIu64 "\n",
              ret, gps.time_utc, gps.timestamp);
     }
   }
   
   static void cp_read_cb(uv_poll_t* handle, int status, int events) {
     struct sensor_gps gps = {};
     int* fd = (int*)(handle->data);
     read(*fd, &gps, sizeof(gps));
     syslog(3, "DEBUG_IN_HELLO, cp recv data:%" PRIu64 ",%" PRIu64 "\n",
            gps.time_utc, gps.timestamp);
   }
   
   void cp_subscriber(void) {
     int sub_fd = orb_subscribe(ORB_ID(sensor_gps));
     syslog(3, "DEBUG_IN_HELLO, cp sub:%d\n", sub_fd);
   
     uv_poll_t handle = {};
     handle.data = &sub_fd;
     uv_poll_init(uv_default_loop(), &handle, sub_fd);
     uv_poll_start(&handle, UV_READABLE, cp_read_cb);
     uv_run(uv_default_loop(), UV_RUN_DEFAULT);
   
   }
   
   struct TopicFD {
     int sub_fd;
     int adv_fd;
   };
   
   static void sensor_read_cb(uv_poll_t* handle, int status, int events) {
     struct sensor_gps gps = {};
     struct TopicFD* fds = (struct TopicFD*)handle->data;
     read(fds->sub_fd, &gps, sizeof(gps));
   
     syslog(3, "DEBUG_IN_HELLO, sen recv data:%" PRIu64 ",%" PRIu64 "\n",
            gps.time_utc, gps.timestamp);
     // need distribute
     if (gps.time_utc == FAKE_CONTROL_CMD) {
       gps.time_utc = FAKE_DISTRIBUTE_CMD;
       int ret = orb_publish(ORB_ID(sensor_gps), fds->adv_fd, &gps);
       syslog(3, "DEBUG_IN_HELLO, sen distribute ctl:%d,%" PRIu64 ",%" PRIu64 "\n",
              ret, gps.time_utc, gps.timestamp);
     }
   }
   
   void sensor_adv_sub_er(void) {
     int instance = 0;
     int adv_fd = orb_advertise_multi_queue(ORB_ID(sensor_gps), NULL, &instance, HELLO_QUEUE_SIZE);
     syslog(3, "DEBUG_IN_HELLO, sen adv:%d\n", adv_fd);
     int sub_fd = orb_subscribe(ORB_ID(sensor_gps));
     syslog(3, "DEBUG_IN_HELLO, sen sub:%d\n", sub_fd);
   
     uv_poll_t handle = {};
     struct TopicFD fds = {};
     fds.sub_fd = sub_fd;
     fds.adv_fd = adv_fd;
     handle.data = &fds;
     uv_poll_init(uv_default_loop(), &handle, sub_fd);
     uv_poll_start(&handle, UV_READABLE, sensor_read_cb);
     uv_run(uv_default_loop(), UV_RUN_DEFAULT);
   }
   ```
   
   1. rpserver and rpproxy open uorb config
   2. build
   ```
   ./build.sh nuttx/boards/sim/sim/sim/configs/rpserver -j16
   ./build.sh nuttx/boards/sim/sim/sim/configs/rpproxy -j16
   ```
   order:
   rpserver:  hello &  and hello 1 &
   rpproxy:  hello 1 1 &
   
   


-- 
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-unsubscribe@nuttx.apache.org

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


Re: [PR] sensor:Fixed the problem of user information lag in cross-core communication "stublist". [nuttx]

Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #13538:
URL: https://github.com/apache/nuttx/pull/13538#discussion_r1768688985


##########
drivers/sensors/sensor.c:
##########
@@ -456,9 +456,17 @@ static ssize_t sensor_do_samples(FAR struct sensor_upperhalf_s *upper,
 
   if (user->state.interval == UINT32_MAX)
     {
-      ret = circbuf_peekat(&upper->buffer,
-                           user->bufferpos * upper->state.esize,
-                           buffer, len);
+      if (buffer != NULL)
+        {
+          ret = circbuf_peekat(&upper->buffer,
+                        user->bufferpos * upper->state.esize,

Review Comment:
   align



-- 
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-unsubscribe@nuttx.apache.org

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


Re: [PR] sensor:Fixed the problem of user information lag in cross-core communication "stublist". [nuttx]

Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 merged PR #13538:
URL: https://github.com/apache/nuttx/pull/13538


-- 
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-unsubscribe@nuttx.apache.org

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


Re: [PR] sensor:Fixed the problem of user information lag in cross-core communication "stublist". [nuttx]

Posted by "Otpvondoiats (via GitHub)" <gi...@apache.org>.
Otpvondoiats commented on code in PR #13538:
URL: https://github.com/apache/nuttx/pull/13538#discussion_r1769480264


##########
drivers/sensors/sensor.c:
##########
@@ -456,9 +456,17 @@ static ssize_t sensor_do_samples(FAR struct sensor_upperhalf_s *upper,
 
   if (user->state.interval == UINT32_MAX)
     {
-      ret = circbuf_peekat(&upper->buffer,
-                           user->bufferpos * upper->state.esize,
-                           buffer, len);
+      if (buffer != NULL)
+        {
+          ret = circbuf_peekat(&upper->buffer,
+                        user->bufferpos * upper->state.esize,

Review Comment:
   Done.



-- 
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-unsubscribe@nuttx.apache.org

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


Re: [PR] sensor:Fixed the problem of user information lag in cross-core communication "stublist". [nuttx]

Posted by "lupyuen (via GitHub)" <gi...@apache.org>.
lupyuen commented on PR #13538:
URL: https://github.com/apache/nuttx/pull/13538#issuecomment-2360105660

   [**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13494&type=pullrequests)
   
   The PR summary is lacking some key information, and the testing section is inadequate. Here's a breakdown:
   
   **What's Missing/Needs Improvement:**
   
   * **Insufficient Detail in Summary:**
       * **What functional part?** Be specific. Is it the uORB messaging layer itself? A specific driver? 
       * **How does the fix work?** Briefly explain the technical approach (e.g., "The fix ensures the stublist is updated by...").
   * **Impact:**
       * **New feature? Existing feature changed?**  A bug fix usually means an existing feature is fixed, but clarify this.
       * **Impact on user?** This bug fix likely impacts users relying on cross-core sensor communication. Explain how their experience might have been affected by the bug and how this fix resolves it.
       * **Impact on documentation?**  If the bug fix reveals a misunderstanding in how the system works, documentation might need updating.
   * **Testing:**
       * **Incomplete Environment:** List the *specific* versions of OS, compiler, target architecture (e.g.,  "Linux Ubuntu 20.04, GCC 9.4.0, SIM:qemu-system-arm").
       * **No "Before" Logs:** Without logs demonstrating the bug, it's impossible to verify the fix.
       * **Unclear "After" Logs:** The provided logs are hard to follow.  Provide clear steps to reproduce the issue and show how the logs now demonstrate the correct behavior after the fix.
   
   **Revised Testing Section (Example):**
   
   ```
   ## Testing
   
   I confirm that changes are verified on the following setup:
   
   * **Build Host:** Ubuntu 20.04, GCC 9.4.0
   * **Target:** SIM: qemu-system-arm (configs: rpserver, rpproxy)
   
   **Steps to Reproduce:**
   
   1. **Before the fix:**
      - Start rpserver: `./nuttx/tools/nsh.sh sim:configs/rpserver`
      - Run: `hello &` and `hello 1 &`
      - Start rpproxy: `./nuttx/tools/nsh.sh sim:configs/rpproxy`
      - Run: `hello 1 1 &`
      - Observe the logs on both rpserver and rpproxy. The bug manifests as [describe the incorrect behavior, e.g., messages not being received, unexpected message overwrites]. 
   
   2. **After the fix:**
      - Repeat steps 1-4.
      - Observe the logs. The fix ensures [describe the expected correct behavior, e.g., messages are now correctly received and processed without overwrites].
   
   **Logs:**
   
   * **Before Fix (trimmed for brevity - attach full logs):**
     ```
     [rpserver logs showing the incorrect behavior]
     [rpproxy logs showing the incorrect behavior] 
     ```
   
   * **After Fix (trimmed for brevity - attach full logs):**
     ```
     [rpserver logs showing the correct behavior]
     [rpproxy logs showing the correct behavior] 
     ``` 
   ```
   
   **Additional Tips:**
   
   * **Conciseness:** Aim for a clear and concise PR description. Avoid overly technical jargon unless necessary.
   * **Focus:** Stick to the essential details of the change.
   * **Completeness:** Don't leave out crucial information that reviewers need to understand and test your changes. 
   


-- 
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-unsubscribe@nuttx.apache.org

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