Skip to content

Conversation

@JordanYates
Copy link
Contributor

Extend the functionality of the emulated GNSS device to support arbitrary reported locations.
Fixes GNSS data callbacks continuing to run after being suspended.
Decouple the reported UTC time from the local system uptime.

Copy link
Contributor

@bjarki-andreasen bjarki-andreasen left a comment

Choose a reason for hiding this comment

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

Have you considered implementing a fake driver instead, which gives you a mock for "free"? Like https://github.com/zephyrproject-rtos/zephyr/blob/main/include/zephyr/drivers/rtc/rtc_fake.h

@JordanYates
Copy link
Contributor Author

Have you considered implementing a fake driver instead, which gives you a mock for "free"? Like https://github.com/zephyrproject-rtos/zephyr/blob/main/include/zephyr/drivers/rtc/rtc_fake.h

At least for me, the point of using the GNSS emulated device is not only to count the number of calls and arguments provided.
The emulator also outputs the periodic fix information in response to those calls.

I really don't see how a "fake" device could pass the test I added, which is basically a requirement for testing any higher level software that wants to use the GNSS API and respond to events it outputs.

@bjarki-andreasen
Copy link
Contributor

bjarki-andreasen commented Oct 17, 2025

Have you considered implementing a fake driver instead, which gives you a mock for "free"? Like https://github.com/zephyrproject-rtos/zephyr/blob/main/include/zephyr/drivers/rtc/rtc_fake.h

At least for me, the point of using the GNSS emulated device is not only to count the number of calls and arguments provided. The emulator also outputs the periodic fix information in response to those calls.

To output the response calls, call gnss_publish() and pass it a struct device pointer, or a fixed value, that can be done without having a driver in the first place. Doing that, combined with the a fake drive,r you can fully emulate concrete behavior across the GNSS API

I really don't see how a "fake" device could pass the test I added, which is basically a requirement for testing any higher level software that wants to use the GNSS API and respond to events it outputs.

The test specifically tests the functionality of the mock implementation added to the emul driver, a fake driver would not need testing itself, its for testing things up against the API, like an application or a shell or something higher level.

@JordanYates
Copy link
Contributor Author

JordanYates commented Oct 17, 2025

To output the response calls, call gnss_publish() and pass it a struct device pointer, or a fixed value, that can be done without having a driver in the first place. Doing that, combined with the a fake drive,r you can fully emulate concrete behavior across the GNSS API

That just seems like a more complicated solution for each test that wants to test against a "real" GNSS device, since it would have to properly manage timing etc. Especially once you start adding in PM behaviour.

But ignoring that, you obviously saw some use for the emulated driver, seeing as you were the one that wrote it originally. Why does it exist if the fake API and gnss_publish are a better solution in your view?

@bjarki-andreasen
Copy link
Contributor

To output the response calls, call gnss_publish() and pass it a struct device pointer, or a fixed value, that can be done without having a driver in the first place. Doing that, combined with the a fake drive,r you can fully emulate concrete behavior across the GNSS API

That just seems like a more complicated solution for each test that wants to test against a "real" GNSS device, since it would have to properly manage timing etc. Especially once you start adding in PM behaviour.

But ignoring that, you obviously saw some use for the emulated driver, seeing as you were the one that wrote it originally. Why does it exist if the fake API and gnss_publish are a better solution in your view?

It was there to test the gnss api test suite itself against a device which acts like a real one, its not supposed to produce specific or correct data, just something that looks like it.

@JordanYates
Copy link
Contributor Author

It was there to test the gnss api test suite itself against a device which acts like a real one, its not supposed to produce specific or correct data, just something that looks like it.

And other subsystems aren't allowed to test themselves again a device that acts like a real GNSS modem?
The suggestion to just call gnss_publish yourself in tests is not helpful, because that just validates that data comes in when the test thinks it should come in (i.e. a useless test).

If you want to test that a piece of code brings up a GNSS device, configures it to some settings, terminates it once some conditions are met, and releases the GNSS device, the "fake + gnss_publish" approach doesn't make sense. The current emulator is 80% of the way there, I don't understand the opposition to making it slightly more flexible.

@bjarki-andreasen
Copy link
Contributor

It was there to test the gnss api test suite itself against a device which acts like a real one, its not supposed to produce specific or correct data, just something that looks like it.

And other subsystems aren't allowed to test themselves again a device that acts like a real GNSS modem? The suggestion to just call gnss_publish yourself in tests is not helpful, because that just validates that data comes in when the test thinks it should come in (i.e. a useless test).

If you want to test that a piece of code brings up a GNSS device, configures it to some settings, terminates it once some conditions are met, and releases the GNSS device, the "fake + gnss_publish" approach doesn't make sense. The current emulator is 80% of the way there, I don't understand the opposition to making it slightly more flexible.

My opposition is based on that the scenario you are describing seems to match "fake + gnss_publish" really well, and that adding a fake implementation is more flexible, extending to more than only the scenario you describe. I want to avoid duplicate and redundant code.

To make my point clear, this PR adds a test suite to test the mock implementations added to gnss_emul, where the mock implementations in the FFF framework are already tested, and provide a common API, so anyone who has used any fake device driver in zephyr already knows how to use them.

I will approve this PR, as its additions don't break anything, and you have a good usecase for it. It may become redundant in the future if someone adds a FFF implementation, we can revisit then in such case.

@kartben kartben requested a review from Copilot October 22, 2025 00:21
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR extends the GNSS emulator driver to support configurable locations and fixes data callback handling during power state transitions. Key changes include decoupling UTC time from system uptime and ensuring callbacks properly stop when the device is suspended.

Key Changes

  • Adds new API functions for manually setting/clearing emulated GNSS data and querying device state regardless of power mode
  • Fixes callback handling by introducing an active flag to prevent callbacks from continuing after device suspension
  • Refactors power management implementation to properly cancel work and clear data on suspend

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
drivers/gnss/gnss_emul.c Core driver implementation with new data setting APIs, refactored PM handling, and UTC time calculation fixes
include/zephyr/drivers/gnss/gnss_emul.h Public API header exposing new emulator control functions
drivers/gnss/Kconfig.emul New configuration option for manual state updates
dts/bindings/gnss/zephyr,gnss-emul.yaml Updated device tree binding to include base.yaml
tests/drivers/gnss/gnss_emul/* Complete test suite validating new functionality and suspend behavior

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Add an option for the GNSS fix state reported by the emulated GNSS modem
to be manually configured by test code, instead of being hardcoded to
always achieve a fix of hardcoded parameters after 5 seconds.

Signed-off-by: Jordan Yates <[email protected]>
Remove the custom initialisation logic that attempts to duplicate
`pm_device_driver_init`.

Signed-off-by: Jordan Yates <[email protected]>
Ensure that callbacks do not continue to fire after `ACTION_SUSPEND`.

Signed-off-by: Jordan Yates <[email protected]>
Provide an escape hatch from the GNSS API requirement that a device
be active to run the configuration `get` functions. This is useful in
the context of an emulator device to query how other software modules
have configured the GNSS.

Signed-off-by: Jordan Yates <[email protected]>
Allow the emulator to be set to an arbitrary UTC time, instead of being
locked to reporting the current system uptime as UTC.

Signed-off-by: Jordan Yates <[email protected]>
Test the behaviour of the emulated GNSS driver.

Signed-off-by: Jordan Yates <[email protected]>
@JordanYates
Copy link
Contributor Author

Comment updates to address Copilot

@sonarqubecloud
Copy link

@jhedberg jhedberg merged commit 95a3110 into zephyrproject-rtos:main Oct 22, 2025
26 checks passed
@JordanYates JordanYates deleted the 251017_gnss_emul branch October 22, 2025 13:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants