Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the testing capabilities for Telco Grand Master (T-GM) clock functionality by introducing a software-based GNSS simulator. This simulator generates NMEA data and provides an API to control signal states, enabling comprehensive and repeatable validation of PTP daemon behavior, DPLL transitions, and event handling in CI environments without reliance on physical GNSS hardware. The changes include new Go application code, Dockerfile, build system integration, and extensive new test suites to leverage this simulation, ultimately improving the reliability and efficiency of T-GM related test cycles. Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a GNSS simulator, a valuable addition for testing T-GM functionality without physical hardware. The implementation is comprehensive, including a Go-based simulator that generates NMEA sentences, a Docker container to run it, and integration into the CI test suite. The code is well-structured and includes good test coverage. I've provided a few suggestions to improve robustness and adhere to best practices, such as pinning the base Docker image version, adding graceful HTTP server shutdown, and ensuring all errors are handled.
edcdavid
left a comment
There was a problem hiding this comment.
Added some comments. Please add a detailed PR description for the new feature. This feature should be integrated with l2 discovery so that any scenario using gnss or telecom GM can discover this gnss.
| value: "$IMG_PREFIX:cep" | ||
| - name: IMAGE_PULL_POLICY | ||
| value: "Always" | ||
| EOF No newline at end of file |
There was a problem hiding this comment.
ptp-tools/scripts/customize-env.sh and ptp-tools/scripts/getoffset.sh are no longer executable, this breaks the tests
test/conformance/serial/ptp.go
Outdated
| Context("Simulated T-GM Verification Tests", func() { | ||
| BeforeEach(func() { | ||
| if !ptphelper.IsSimulatedTGM() { | ||
| Skip("test valid only for simulated T-GM (PTP_TEST_MODE=tgm-sim)") |
There was a problem hiding this comment.
We should not create a new tgm-sim, the existing tgm scenario should already cover it by discovering the WPC card that provides the gnss hardware. The simulator should appear to the script as real HW so that ptp-operator performs as if on real hardware. The virtual NIC supporting gnss should have a pci ID that will make it be discovered as gnrd (supporting gnss)
Summary
Adds a software GNSS simulator to enable T-GM (Telco Grand Master) end-to-end testing in CI without physical GNSS receivers, Intel WPC NICs, or hardware DPLL.
Motivation / Problem
JIRA Ticket
The existing T-GM test suite (
PTP_TEST_MODE=tgm) requires hardware that is unavailable in the Kind-cluster-based CI environment:000e)/dev/gnssubxtoolThis blocks upstream CI coverage for T-GM clock state verification, holdover scenarios, and PTP event API validation.
Solution
A new
ptp-tools/gnss-sim/Go binary generates valid NMEA sentences (GNRMC, GNGGA, GPZDA) at 1 PPS over socat virtual serial ports. It includes:/api/signal/lossand/api/signal/restorereplaceubxtoolcommands for test orchestrationts2phcreads from/dev/ttyGNSS_TS2PHCas if it were a real GNSS deviceA new test mode
PTP_TEST_MODE=tgm-simactivates the simulation. WPC NIC detection functions are mocked at the test helper level soCreatePtpConfigWPCGrandMaster()— the real config creation path — runs unchanged. The only conditional is skipping the E810 plugin (pin maps, ubxtool) since netdevsim has no Intel ICE driver.Technical Details
Architecture decision — mock detection, not config creation:
Rather than creating a separate sim-specific config function, the detection layer (
getWPCEnabledIfaces,checkGNSSAvailabilityForIface) returns simulated data whenIsSimulatedTGM()is true. This keeps the config creation path identical to production. L2 discovery is bypassed because netdevsim interfaces cannot satisfy the solver'sStepIsWPCNicconstraint.Key files:
ptp-tools/gnss-sim/— main, nmea, simulator, dpll, api, entrypoint, unit testsptp-tools/Dockerfile.gnss-sim,scripts/configGNSS.shtest/pkg/ptphelper/ptphelper.gotest/pkg/testconfig/testconfig.gotest/conformance/serial/ptp.go— 3 new contexts (verification, signal loss, CloudEvents v2)scripts/run-on-vm.sh,scripts/run-tests.sh,ptp-tools/MakefileBreaking changes: None. All changes are additive; existing test modes are unaffected.
New dependencies: None. The simulator is a standalone Go binary; the only runtime dependency is
socat(installed in the container image).Testing
make fmt+go vet: clean--focus=".*Simulated T-GM.*"PTP_TEST_MODE=tgm-simRisks / Considerations
ptphelper.godetection mocks —getWPCEnabledIfaces()andcheckGNSSAvailabilityForIface()have early-return branches whenIsSimulatedTGM(). Verify these don't affect realtgmmode (they are gated onPTP_TEST_MODE=tgm-simonly).CreatePtpConfigWPCGrandMaster()passesnilplugins in sim mode. Confirm this doesn't breakcreateConfigWithTs2PhcAndPlugins()when plugins are nil.ts2phc,phc2sys, andptp4l.Reviewer Checklist
Assisted-by AI tool