Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sensor:Fixed the problem of user information lag in cross-core communication "stublist". #13538

Merged
merged 1 commit into from
Sep 21, 2024

Conversation

Otpvondoiats
Copy link

@Otpvondoiats Otpvondoiats commented Sep 19, 2024

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);
}

PC: ubuntu 2004,x86,SIM

  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 &

@lupyuen
Copy link
Member

lupyuen commented Sep 19, 2024

[Experimental Bot, please feedback here]

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.

@github-actions github-actions bot added the Size: S The size of the change in this PR is small label Sep 19, 2024
@Otpvondoiats Otpvondoiats force-pushed the lk_fix_sensor_stublist branch 2 times, most recently from 9ac2737 to 147e5fa Compare September 20, 2024 10:21
…nication "stublist".

When its remote core publishes a message, all subscribed cores will receive the message,
but the local core "stublist" does not update the user's "generation" and "bufferpos" parameters.

Signed-off-by: likun17 <[email protected]>
@xiaoxiang781216 xiaoxiang781216 merged commit 09ab135 into apache:master Sep 21, 2024
29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Sensors Sensors issues Size: S The size of the change in this PR is small
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants