Skip to content

Consistently serialize neural network data#2224

Merged
samfreund merged 1 commit intoPhotonVision:mainfrom
Gold856:fix-od-serde
Dec 7, 2025
Merged

Consistently serialize neural network data#2224
samfreund merged 1 commit intoPhotonVision:mainfrom
Gold856:fix-od-serde

Conversation

@Gold856
Copy link
Member

@Gold856 Gold856 commented Dec 7, 2025

Description

#1894 implemented custom serialization for Path handling that's used to serialize NN settings into the SQLite database, which just saved the raw Path and avoiding file: URIs. However, this causes a rift between what's sent to the client, and what's actually stored in the config because the custom serialization is only used for saving into the config. This wasn't an issue before, since at worst, there were some extra slashes, since the URI typically looked like file:///etc/..., but only file: was dropped on the client end and the extra slashes are just dropped when constructing a new Path object, but with the advent of Playwright testing, model paths can involve Windows paths, leaving paths like //C:/path/... which Path.of cannot decode. We could just drop file:// and that would likely work, but having this kind of inconsistent serde is concerning. So we're just going to use Jackson's standard path serialization for consistency, because it will handle it all seamlessly (and we don't have to convert Strings to Paths ourselves). This undoes the changes made to JacksonUtils in #1894.

Meta

Merge checklist:

  • Pull Request title is short, imperative summary of proposed changes
  • The description documents the what and why
  • If this PR changes behavior or adds a feature, user documentation is updated
  • If this PR touches photon-serde, all messages have been regenerated and hashes have not changed unexpectedly
  • If this PR touches configuration, this is backwards compatible with settings back to v2025.3.2
  • If this PR touches pipeline settings or anything related to data exchange, the frontend typing is updated
  • If this PR addresses a bug, a regression test for it is added

@Gold856 Gold856 requested a review from a team as a code owner December 7, 2025 05:52
@github-actions github-actions bot added frontend Having to do with PhotonClient and its related items backend Things relating to photon-core and photon-server labels Dec 7, 2025
samfreund
samfreund previously approved these changes Dec 7, 2025
@samfreund samfreund merged commit 6285f1e into PhotonVision:main Dec 7, 2025
42 checks passed
@Gold856 Gold856 deleted the fix-od-serde branch December 9, 2025 17:34
Gold856 added a commit that referenced this pull request Dec 9, 2025
## Description

#2224 removed the custom deserializers for `Path`, but we still need one
to be able to deserialize the `Path` key in
`NeuralNetworkPropertyManager`. Additionally, Jackson seems to
auto-convert the `Path` key to a `String` using `toString` instead of
its own serializers, so a custom key serializer is also needed to
consistently use the same format for paths. This also removes unused
serde methods in `JacksonUtils` to minimize potential future churn, and
tacks an `@JsonIgnore` on `getModels` to prevent Jackson from
serializing a `ModelProperty` array into the database.

## Meta

Merge checklist:
- [x] Pull Request title is [short, imperative
summary](https://cbea.ms/git-commit/) of proposed changes
- [x] The description documents the _what_ and _why_
- [ ] If this PR changes behavior or adds a feature, user documentation
is updated
- [ ] If this PR touches photon-serde, all messages have been
regenerated and hashes have not changed unexpectedly
- [ ] If this PR touches configuration, this is backwards compatible
with settings back to v2025.3.2
- [ ] If this PR touches pipeline settings or anything related to data
exchange, the frontend typing is updated
- [x] If this PR addresses a bug, a regression test for it is added
samfreund pushed a commit to samfreund/photonvision that referenced this pull request Dec 18, 2025
samfreund pushed a commit to samfreund/photonvision that referenced this pull request Dec 18, 2025
…Vision#2232)

## Description

PhotonVision#2224 removed the custom deserializers for `Path`, but we still need one
to be able to deserialize the `Path` key in
`NeuralNetworkPropertyManager`. Additionally, Jackson seems to
auto-convert the `Path` key to a `String` using `toString` instead of
its own serializers, so a custom key serializer is also needed to
consistently use the same format for paths. This also removes unused
serde methods in `JacksonUtils` to minimize potential future churn, and
tacks an `@JsonIgnore` on `getModels` to prevent Jackson from
serializing a `ModelProperty` array into the database.

## Meta

Merge checklist:
- [x] Pull Request title is [short, imperative
summary](https://cbea.ms/git-commit/) of proposed changes
- [x] The description documents the _what_ and _why_
- [ ] If this PR changes behavior or adds a feature, user documentation
is updated
- [ ] If this PR touches photon-serde, all messages have been
regenerated and hashes have not changed unexpectedly
- [ ] If this PR touches configuration, this is backwards compatible
with settings back to v2025.3.2
- [ ] If this PR touches pipeline settings or anything related to data
exchange, the frontend typing is updated
- [x] If this PR addresses a bug, a regression test for it is added
spacey-sooty pushed a commit to spacey-sooty/photonvision that referenced this pull request Dec 22, 2025
spacey-sooty pushed a commit to spacey-sooty/photonvision that referenced this pull request Dec 22, 2025
…Vision#2232)

## Description

PhotonVision#2224 removed the custom deserializers for `Path`, but we still need one
to be able to deserialize the `Path` key in
`NeuralNetworkPropertyManager`. Additionally, Jackson seems to
auto-convert the `Path` key to a `String` using `toString` instead of
its own serializers, so a custom key serializer is also needed to
consistently use the same format for paths. This also removes unused
serde methods in `JacksonUtils` to minimize potential future churn, and
tacks an `@JsonIgnore` on `getModels` to prevent Jackson from
serializing a `ModelProperty` array into the database.

## Meta

Merge checklist:
- [x] Pull Request title is [short, imperative
summary](https://cbea.ms/git-commit/) of proposed changes
- [x] The description documents the _what_ and _why_
- [ ] If this PR changes behavior or adds a feature, user documentation
is updated
- [ ] If this PR touches photon-serde, all messages have been
regenerated and hashes have not changed unexpectedly
- [ ] If this PR touches configuration, this is backwards compatible
with settings back to v2025.3.2
- [ ] If this PR touches pipeline settings or anything related to data
exchange, the frontend typing is updated
- [x] If this PR addresses a bug, a regression test for it is added
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend Things relating to photon-core and photon-server frontend Having to do with PhotonClient and its related items

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants