Skip to content

Update hopsAway when packets are received #988

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
esev wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Update hopsAway when packets are received #988

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions packages/web/src/core/stores/nodeDBStore/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -270,12 +270,18 @@ function nodeDBFactory(
}
const node = nodeDB.nodeMap.get(data.from);
const nowSec = Math.floor(Date.now() / 1000); // lastHeard is in seconds(!)
const hopsAway =
data.hopLimit > data.hopStart ||
(data.hopStart === 0 && !data.hasBitfield)
? undefined
: data.hopStart - data.hopLimit;

if (node) {
const updated = {
...node,
lastHeard: data.time > 0 ? data.time : nowSec,
snr: data.snr,
hopsAway,
};
nodeDB.nodeMap = new Map(nodeDB.nodeMap).set(data.from, updated);
} else {
Expand All @@ -285,6 +291,7 @@ function nodeDBFactory(
num: data.from,
lastHeard: data.time > 0 ? data.time : nowSec, // fallback to now if time is 0 or negative,
snr: data.snr,
hopsAway,
}),
);
}
Expand Down
35 changes: 30 additions & 5 deletions packages/web/src/core/stores/nodeDBStore/nodeDBStore.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,18 +95,43 @@ describe("NodeDB store", () => {
const { useNodeDBStore } = await freshStore();
const db = useNodeDBStore.getState().addNodeDB(1);

db.processPacket({ from: 50, time: 1111, snr: 7 } as any);
db.processPacket({
from: 50,
time: 1111,
snr: 7,
hopStart: 5,
hopLimit: 2,
hasBitfield: true,
} as any);
Comment on lines +98 to +105
Copy link

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

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

These processPacket(...) calls now satisfy ProcessPacketParams, so the as any casts are no longer needed. Dropping them will keep the test type-safe and prevent accidentally omitting fields in future updates.

Copilot uses AI. Check for mistakes.
expect(db.getNode(50)).toBeTruthy();
expect(db.getNode(50)?.lastHeard).toBe(1111);
expect(db.getNode(50)?.snr).toBe(7);

db.processPacket({ from: 50, time: 2222, snr: 9 } as any);
expect(db.getNode(50)?.hopsAway).toBe(3);

db.processPacket({
from: 50,
time: 2222,
snr: 9,
hopStart: 6,
hopLimit: 3,
hasBitfield: true,
} as any);
expect(db.getNode(50)?.lastHeard).toBe(2222);
expect(db.getNode(50)?.snr).toBe(9);

db.processPacket({ from: 50, time: 0, snr: 9 } as any);
expect(db.getNode(50)?.hopsAway).toBe(3);

// when hopStart===0 and hasBitfield is false, hopsAway should be undefined
db.processPacket({
from: 50,
time: 0,
snr: 9,
hopStart: 0,
hopLimit: 0,
hasBitfield: false,
} as any);
expect(db.getNode(50)?.lastHeard).toBeCloseTo(Date.now() / 1000, -1); // within 1s, note lastHeard is in seconds
Copy link

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

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

This assertion comment says “within 1s”, but toBeCloseTo(..., -1) allows a ~5 second delta. Either adjust the numDigits (or use fake timers/vi.setSystemTime) to match the intended tolerance, or update the comment to reflect the actual tolerance.

Suggested change
expect(db.getNode(50)?.lastHeard).toBeCloseTo(Date.now() / 1000, -1); // within 1s, note lastHeard is in seconds
expect(db.getNode(50)?.lastHeard).toBeCloseTo(Date.now() / 1000, -1); // approximate current time in seconds, with broad tolerance

Copilot uses AI. Check for mistakes.
expect(db.getNode(50)?.snr).toBe(9);
expect(db.getNode(50)?.hopsAway).toBeUndefined();
Copy link

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

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

The new hopsAway disambiguation logic has a few key branches, but the test only covers the hopStart===0 && !hasBitfield (undefined) path. Please add assertions for at least: (1) hopStart===0 && hasBitfield===true (should yield hopsAway === 0), and (2) hopLimit > hopStart (should yield undefined).

Suggested change
expect(db.getNode(50)?.hopsAway).toBeUndefined();
expect(db.getNode(50)?.hopsAway).toBeUndefined();
// when hopStart===0 and hasBitfield is true, hopsAway should be 0
db.processPacket({
from: 50,
time: 3333,
snr: 10,
hopStart: 0,
hopLimit: 0,
hasBitfield: true,
} as any);
expect(db.getNode(50)?.lastHeard).toBe(3333);
expect(db.getNode(50)?.snr).toBe(10);
expect(db.getNode(50)?.hopsAway).toBe(0);
// when hopLimit > hopStart, hopsAway should be undefined
db.processPacket({
from: 50,
time: 4444,
snr: 11,
hopStart: 1,
hopLimit: 2,
hasBitfield: true,
} as any);
expect(db.getNode(50)?.lastHeard).toBe(4444);
expect(db.getNode(50)?.snr).toBe(11);
expect(db.getNode(50)?.hopsAway).toBeUndefined();

Copilot uses AI. Check for mistakes.
});

it("addUser and addPosition updates existing or creates new nodes", async () => {
Expand Down
3 changes: 3 additions & 0 deletions packages/web/src/core/stores/nodeDBStore/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ type ProcessPacketParams = {
from: number;
snr: number;
time: number;
hopStart: number;
hopLimit: number;
hasBitfield: boolean;
Copy link

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

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

hasBitfield is ambiguous (which bitfield/where?) and is being used specifically as a packet capability/version signal. Consider renaming it to something more explicit like hasPayloadBitfield / hasDecodedBitfield (or similar) so it’s clear it refers to the decoded payload’s optional bitfield presence, not a generic node property.

Suggested change
hasBitfield: boolean;
hasPayloadBitfield: boolean;

Copilot uses AI. Check for mistakes.
};

export type { NodeError, ProcessPacketParams, NodeErrorType };
5 changes: 5 additions & 0 deletions packages/web/src/core/subscriptions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,11 @@ export const subscribeAll = (
from: meshPacket.from,
snr: meshPacket.rxSnr,
time: meshPacket.rxTime,
hopStart: meshPacket.hopStart,
hopLimit: meshPacket.hopLimit,
hasBitfield:
meshPacket.payloadVariant.case === "decoded" &&
meshPacket.payloadVariant.value.bitfield !== undefined,
});
});

Expand Down