Skip to content

support for device custom property intervals#153

Merged
brookfxg merged 11 commits intomainfrom
brook/fg-14094-update-foxglove-python-with-device-property-history
Mar 30, 2026
Merged

support for device custom property intervals#153
brookfxg merged 11 commits intomainfrom
brook/fg-14094-update-foxglove-python-with-device-property-history

Conversation

@brookfxg
Copy link
Copy Markdown
Contributor

@brookfxg brookfxg commented Mar 24, 2026

Changelog

Support for device custom property time intervals

Docs

Relevant docs will be updated with the associated API PR

Description

Pending

Adds support for device custom property time intervals - listing, getting, and updating. Also a small deduplication refactor to consolidate our device id or name handling into a single method. This is in line with how we do session id or key.

Testing

Local testing against a local API instance. A custom test script was used to hit the API with a high volume of reads and edits and examine the results for correctness.

Screenshot 2026-03-24 at 1 40 08 PM

@linear
Copy link
Copy Markdown

linear bot commented Mar 24, 2026

claude[bot]

This comment was marked as outdated.

claude[bot]

This comment was marked as outdated.

claude[bot]

This comment was marked as outdated.

@brookfxg brookfxg changed the title support for device custom property history support for device custom property intervals Mar 25, 2026
claude[bot]

This comment was marked as outdated.

claude[bot]

This comment was marked as outdated.

claude[bot]

This comment was marked as outdated.

Keep the update property time interval tests consistent with the singular client method name so the renamed API surface is less confusing in review.

Made-with: Cursor
Copy link
Copy Markdown
Contributor

@claude claude bot left a comment

Choose a reason for hiding this comment

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

All prior inline threads resolved. The _device_identifier extraction is a nice dedup, and the urlquote addition to the existing device methods fixes path-encoding for names with special characters — that's a good side-effect improvement.

One note: the _device_identifier refactor changes the existing validation from truthy checks (if device_name and device_id) to is not None checks. This means callers who previously passed device_name="" alongside a device_id wouldn't have gotten the mutual-exclusivity error, but now will. More correct behavior, but technically a subtle breaking change for get_device, update_device, and delete_device.

PR description still has "Pending" in the Description section — clean that up before un-drafting.

"limit": ["5"],
"offset": ["10"],
"projectId": ["project-id"],
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

suggestion: The list test covers key, limit, offset, and projectId but doesn't exercise start/end. Those params go through astimezone().isoformat() serialization, which is the most interesting codepath to verify (timezone handling, format). Worth adding a test that passes start/end datetimes and asserts the query params are serialized correctly.

@brookfxg brookfxg marked this pull request as ready for review March 30, 2026 17:12
@brookfxg brookfxg requested a review from mguida22 as a code owner March 30, 2026 17:12
…rty-history' of github.com:foxglove/foxglove-python into brook/fg-14094-update-foxglove-python-with-device-property-history
Copy link
Copy Markdown
Contributor

@claude claude bot left a comment

Choose a reason for hiding this comment

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

Prior inline threads all resolved except the start/end test coverage suggestion — still worth adding.

Two new questions on the update method (see inline): projectId placement (body vs query param) breaks from the pattern every other device endpoint uses, and the 204/else branching looks like it might have a dead code path.

mguida22's suggestion to combine _device_identifier + urlquote into a single helper is still open — Brook's proposed _device_identifier_with_path sounds reasonable, worth doing before merge to avoid the repetition across 6 call sites.

PR description still has "Pending" in the Description section.

@brookfxg brookfxg merged commit 845ce2d into main Mar 30, 2026
5 checks passed
@brookfxg brookfxg deleted the brook/fg-14094-update-foxglove-python-with-device-property-history branch March 30, 2026 20:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants