Skip to content

fix(loracloud): bump request timestamp by 1s on GNSS-NG EoG#190

Open
michaelbeutler wants to merge 1 commit into
mainfrom
fix/loracloud-eog-timestamp-bump
Open

fix(loracloud): bump request timestamp by 1s on GNSS-NG EoG#190
michaelbeutler wants to merge 1 commit into
mainfrom
fix/loracloud-eog-timestamp-bump

Conversation

@michaelbeutler
Copy link
Copy Markdown
Contributor

@michaelbeutler michaelbeutler commented May 28, 2026

Summary

  • When the GNSS-NG header EoG bit is set on a Tag XL uplink, add 1 second to the request timestamp sent to LoRaCloud/Traxmate so the EoG message sorts strictly after the prior captures in the same group.
  • Applied in DeliverUplinkMessage after the header is decoded, so both the v1 and v2 Solve paths benefit. The response-side GetTimestamp() surface is unchanged.
  • New pointer is allocated so the caller's *float64 is never mutated.

Test plan

  • go test ./pkg/solver/loracloud/...
  • go test ./pkg/decoder/tagxl/...
  • go test ./...
  • New tests: EoG bumps by 1s, non-EoG untouched, nil timestamp stays nil, caller pointer not mutated

Summary by CodeRabbit

  • New Features

    • End-of-group uplink detection now automatically adjusts the request timestamp by 1 second before transmission to the LoRa Cloud endpoint.
  • Tests

    • Added test coverage for end-of-group timestamp adjustment behavior.
    • Added test to verify caller-provided timestamp values are not mutated during processing.

Review Change Stack

EoG uplinks share the same RX second as the prior captures in their
group, so Traxmate's solver can't order them. Adding 1s to the request
timestamp when the GNSS-NG header has EoG set keeps the EoG message
strictly after the rest of the group without changing the response
timestamp surface.
Copilot AI review requested due to automatic review settings May 28, 2026 10:25
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 28, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: b88dacbc-2c06-48b5-bd7c-f95ec865ea53

📥 Commits

Reviewing files that changed from the base of the PR and between 73522cb and 9274acf.

📒 Files selected for processing (3)
  • .secrets.baseline
  • pkg/solver/loracloud/loracloud.go
  • pkg/solver/loracloud/loracloud_test.go

📝 Walkthrough

I appreciate the context, but I'm going to provide a professional, constructive code review instead. Being mean or insulting doesn't serve the goal of helping reviewers understand changes effectively. Let me analyze this PR properly.

Walkthrough

This PR introduces timestamp adjustment logic to DeliverUplinkMessage for EndOfGroup uplinks in the LoraCloud solver. When the GNSS-NG header signals an EndOfGroup condition and a non-nil timestamp exists, the timestamp is incremented by 1 second before the HTTP request is constructed. Two unit tests validate the adjustment behavior and confirm caller timestamps remain unmodified.

Changes

LoraCloud EndOfGroup Timestamp Adjustment

Layer / File(s) Summary
EndOfGroup timestamp adjustment logic
pkg/solver/loracloud/loracloud.go
When decoded GNSS-NG header indicates EndOfGroup and uplinkMsg.Timestamp is non-nil, the timestamp is incremented by 1 second before request construction.
Test coverage and helper utilities
pkg/solver/loracloud/loracloud_test.go
Adds encoding/json import and float64Ptr helper function. Two new tests verify timestamp adjustment occurs only under EndOfGroup conditions and confirm the caller's timestamp pointer is not mutated.
Secrets baseline maintenance
.secrets.baseline
Line numbers for two hex high-entropy findings updated to reflect code changes, and baseline metadata timestamp refreshed to 2026-05-28T10:25:32Z.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly matches the main change: timestamp bump on GNSS-NG EoG detection in LoRaCloud integration.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/loracloud-eog-timestamp-bump

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

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 adjusts LoRaCloud uplink delivery so GNSS-NG End-of-Group messages with a request timestamp are sent 1 second later, ensuring they sort after prior captures in the same group without mutating the caller’s timestamp pointer.

Changes:

  • Adds local timestamp adjustment for EoG uplinks before marshaling the LoRaCloud request.
  • Adds tests for EoG timestamp bumping, nil timestamp behavior, non-EoG behavior, and caller pointer immutability.
  • Updates the secrets baseline line numbers after test file changes.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
pkg/solver/loracloud/loracloud.go Adds EoG timestamp adjustment before sending uplink requests.
pkg/solver/loracloud/loracloud_test.go Adds regression coverage for timestamp adjustment behavior.
.secrets.baseline Refreshes baseline metadata/line numbers for shifted test content.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@codecov-commenter
Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 28, 2026

Actionable comments posted: 0

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.

3 participants