Skip to content

uorb:Update uorb info and uorb topic#2603

Merged
xiaoxiang781216 merged 2 commits intoapache:masterfrom
Otpvondoiats:lk_sensor
Sep 27, 2024
Merged

uorb:Update uorb info and uorb topic#2603
xiaoxiang781216 merged 2 commits intoapache:masterfrom
Otpvondoiats:lk_sensor

Conversation

@Otpvondoiats
Copy link
Copy Markdown
Contributor

@Otpvondoiats Otpvondoiats commented Sep 26, 2024

Summary

  1. uorb adds interface for getting info
  2. Add some sensor types from aosp( 6dof motion and gesture related types).

Impact

n/a

Testing

  • Environment:

OS and Version: Ubuntu 20.04.6 LTS x86_64
GCC Version: 13.1.0
SIM: ./tools/configure.sh sim:nsh

  • Order:

./nuttx

@Otpvondoiats Otpvondoiats changed the title Lk sensor uorb:Update sensor type Sep 26, 2024
@Otpvondoiats Otpvondoiats changed the title uorb:Update sensor type uorb:Update uorb topic Sep 26, 2024
@Otpvondoiats Otpvondoiats changed the title uorb:Update uorb topic uorb:Update uorb info and uorb topic Sep 26, 2024
@nuttxpr
Copy link
Copy Markdown

nuttxpr commented Sep 26, 2024

[Experimental Bot, please feedback here]

The provided PR information partially meets the NuttX requirements. Here's a breakdown:

What's Good:

  • Summary: It outlines the changes (adding a uorb interface and sensor types). However, it could be more descriptive about why these changes are necessary.
  • Testing: It provides environment details and basic testing steps.

What Needs Improvement:

  • Summary:
    • Missing "Why": Explain the rationale behind adding the uorb interface and the specific sensor types. What problem does this solve? What new functionality does it enable?
    • Lack of Detail: Be more specific about the "uorb interface" – what information does it retrieve? Similarly, be more precise about the added sensor types (e.g., "Added support for accelerometer, gyroscope, and significant motion detection from AOSP").
    • References: If this PR addresses any existing issues in the NuttX or NuttX Apps repositories, provide links to those issues.
  • Impact: The "n/a" response is too broad. Carefully consider each impact category and provide specific details:
    • New Feature: Clearly state that this PR introduces a new feature.
    • User Impact: Will users need to modify their applications or configurations to use this new interface and sensor types?
    • Build Impact: Are there any changes to the build system, new dependencies, or configuration options?
    • Hardware Impact: Does this PR target specific architectures or boards? Does it introduce support for new sensors?
    • Documentation Impact: Does this PR require updates to the NuttX documentation? If so, are these updates included in the PR?
    • Security Impact: Do these changes introduce any potential security vulnerabilities?
    • Compatibility Impact: Does this PR maintain backward compatibility? Are there any implications for forward compatibility or interoperability with other systems?
  • Testing:
    • Logs: Instead of just stating "./nuttx," provide relevant snippets from the testing logs that demonstrate the functionality before and after the changes. Highlight the key differences the changes introduced.
    • Test Cases: Consider adding more specific test cases that validate:
      • The uorb interface is accessible and returns the expected data.
      • The new sensor types are correctly recognized and their data can be retrieved.

In summary: The PR needs more details and context to meet the NuttX requirements fully. Address the missing information and expand on the existing sections to make your PR informative and easy to review.

@cederom
Copy link
Copy Markdown
Contributor

cederom commented Sep 26, 2024

accept4 related build error:

nettest_server.c: In function 'nettest_server':
Warning: nettest_server.c:151:14: warning: implicit declaration of function 'accept4'; did you mean 'accept'? [-Wimplicit-function-declaration]

@xiaoxiang781216
Copy link
Copy Markdown
Contributor

accept4 related build error:

nettest_server.c: In function 'nettest_server':
Warning: nettest_server.c:151:14: warning: implicit declaration of function 'accept4'; did you mean 'accept'? [-Wimplicit-function-declaration]

@cederom where do you find the above warning? All error I found from uorb related files from:
https://github.com/apache/nuttx-apps/actions/runs/11050874650/job/30735671643?pr=2603

@Otpvondoiats
Copy link
Copy Markdown
Contributor Author

depend on nuttx apache/nuttx#13639

@cederom
Copy link
Copy Markdown
Contributor

cederom commented Sep 27, 2024

accept4 related build error:

nettest_server.c: In function 'nettest_server':
Warning: nettest_server.c:151:14: warning: implicit declaration of function 'accept4'; did you mean 'accept'? [-Wimplicit-function-declaration]

@cederom where do you find the above warning? All error I found from uorb related files from: https://github.com/apache/nuttx-apps/actions/runs/11050874650/job/30735671643?pr=2603

sorry, this is warning, errors come from unknown structs. i need to get some sleep :-P

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants