Skip to content

sensors: gnss buffer usage optimalisation#657

Draft
niewim19 wants to merge 1 commit into
masterfrom
niewim/gnssbuf
Draft

sensors: gnss buffer usage optimalisation#657
niewim19 wants to merge 1 commit into
masterfrom
niewim/gnssbuf

Conversation

@niewim19

Copy link
Copy Markdown
Contributor
  • increase queue size to fully cover buffer
  • return "updated" only on GGA messages (they contain new fix)

JIRA: PP-449

Description

Motivation and Context

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Chore (refactoring, style fixes, git/CI config, submodule management, no code logic changes)

How Has This Been Tested?

  • Already covered by automatic testing.
  • New test added: (add PR link here).
  • Tested by hand on: (list targets here).

Checklist:

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing linter checks and tests passed.
  • My changes generate no new compilation warnings for any of the targets.

Special treatment

  • This PR needs additional PRs to work (list the PRs, preferably in merge-order).
  • I will merge this PR by myself when appropriate.

 - increase queue size to fully cover buffer
 - return "updated" only on GGA messages (they contain new fix)

JIRA: PP-449

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request modifies the GPS event update logic to return status codes directly from switch cases and increases the UBX inbox size. It also adds a new UBX configuration command to set UART1 output to NMEA only at 115200 baud. A review comment points out that the write call for this new command incorrectly includes the null terminator from the string literal, which may cause issues with the binary protocol; it is recommended to use sizeof(...) - 1 instead.

Comment thread sensors/gps/ubx.c
write(ctx->filedes, UBX_PREMADE_FIX_10HZ, sizeof(UBX_PREMADE_FIX_10HZ));
usleep(100 * 1000);

write(ctx->filedes, UBX_PREMADE_UART1_OUT_NMEA_ONLY_115200, sizeof(UBX_PREMADE_UART1_OUT_NMEA_ONLY_115200));

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Using sizeof() on a string literal includes the implicit null terminator (\0). Since UBX is a binary protocol, sending an extra 0x00 byte after the message is incorrect and may be misinterpreted by the receiver. It is recommended to use sizeof(...) - 1 to send only the actual message bytes.

write(ctx->filedes, UBX_PREMADE_UART1_OUT_NMEA_ONLY_115200, sizeof(UBX_PREMADE_UART1_OUT_NMEA_ONLY_115200) - 1);

@github-actions

Copy link
Copy Markdown

Unit Test Results

10 514 tests   - 29   9 854 ✅  - 27   55m 13s ⏱️ + 2m 1s
   649 suites ± 0     658 💤  -  4 
     1 files   ± 0       2 ❌ + 2 

For more details on these failures, see this check.

Results for commit e5cd178. ± Comparison against base commit f20ef15.

This pull request removes 30 and adds 1 tests. Note that renamed tests count towards both.
phoenix-rtos-tests/libc/exit ‑ aarch64a53-zynqmp-qemu:phoenix-rtos-tests/libc/exit.stdlib_exit.atexit_few_calls
phoenix-rtos-tests/libc/exit ‑ aarch64a53-zynqmp-qemu:phoenix-rtos-tests/libc/exit.stdlib_exit.atexit_register_inside
phoenix-rtos-tests/libc/exit ‑ aarch64a53-zynqmp-qemu:phoenix-rtos-tests/libc/exit.stdlib_exit.atexit_two_nodes
phoenix-rtos-tests/libc/exit ‑ aarch64a53-zynqmp-qemu:phoenix-rtos-tests/libc/exit.stdlib_exit.stream_flush
phoenix-rtos-tests/libc/exit ‑ aarch64a53-zynqmp-qemu:phoenix-rtos-tests/libc/exit.unistd_Exit.SIGCHLD_sent
phoenix-rtos-tests/libc/exit ‑ aarch64a53-zynqmp-qemu:phoenix-rtos-tests/libc/exit.unistd_Exit.chk_if_exits
phoenix-rtos-tests/libc/exit ‑ aarch64a53-zynqmp-qemu:phoenix-rtos-tests/libc/exit.unistd_Exit.close_streams
phoenix-rtos-tests/libc/exit ‑ aarch64a53-zynqmp-qemu:phoenix-rtos-tests/libc/exit.unistd_Exit.exit_status_waitpid
phoenix-rtos-tests/libc/exit ‑ aarch64a53-zynqmp-qemu:phoenix-rtos-tests/libc/exit.unistd_Exit.new_parent_id
phoenix-rtos-tests/libc/exit ‑ aarch64a53-zynqmp-qemu:phoenix-rtos-tests/libc/exit.unistd_Exit.no_atexit
…
phoenix-rtos-tests/libc/exit ‑ aarch64a53-zynqmp-qemu:phoenix-rtos-tests/libc/exit

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant