Skip to content

Add missing early-return to legacy ML model load#2347

Merged
mcm001 merged 5 commits intoPhotonVision:mainfrom
samfreund:oops
Feb 2, 2026
Merged

Add missing early-return to legacy ML model load#2347
mcm001 merged 5 commits intoPhotonVision:mainfrom
samfreund:oops

Conversation

@samfreund
Copy link
Member

@samfreund samfreund commented Feb 2, 2026

Description

Previously, when attempting to load a model that did not follow the old naming pattern that was in the models directory, we caused PV to crash due to a missing early return (reproducable on this branch with the extra bonus ML model and missing label file):

    [2026-02-01 17:35:15] [Config - NeuralNetworkModelManager] [ERROR] Model properties are null. This could mean the config for model /home/matth/photonvision/test-resources/old_configs/2025.3.1-old-nnmm/models/iCauseProblems.rknn was unable to be found in the database. Trying legacy...
    [2026-02-01 17:35:15] [Config - NeuralNetworkModelManager] [ERROR] Failed to translate legacy model filename to properties: /home/matth/photonvision/test-resources/old_configs/2025.3.1-old-nnmm/models/iCauseProblems.rknn: /home/matth/photonvision/test-resources/old_configs/2025.3.1-old-nnmm/models/iCauseProblems-labels.txt

SQLConfigTest > testLoadNewNNMM() FAILED
    java.lang.NullPointerException: Cannot invoke "org.photonvision.common.configuration.NeuralNetworkModelsSettings$ModelProperties.toString()" because "properties" is null
        at org.photonvision.common.configuration.NeuralNetworkModelManager.loadModel(NeuralNetworkModelManager.java:342)
        at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.accept(ForEachOps.java:183)

This PR fixes it by adding the missing early-return, and adding unit tests to make sure we handle this.

Meta

Merge checklist:

  • Pull Request title is short, imperative summary of proposed changes
  • The description documents the what and why, including events that led to this PR
  • 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 all settings going back to the previous seasons's last release (seasons end after champs ends)
  • 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

@samfreund samfreund requested a review from a team as a code owner February 2, 2026 00:55
@github-actions github-actions bot added the backend Things relating to photon-core and photon-server label Feb 2, 2026
@mcm001 mcm001 changed the title Fix OD migration behavior for new models not in database Add missing early-return to legacy ML model load Feb 2, 2026
@mcm001 mcm001 enabled auto-merge (squash) February 2, 2026 01:48
@mcm001 mcm001 merged commit 23392f8 into PhotonVision:main Feb 2, 2026
58 checks passed
@samfreund samfreund deleted the oops branch February 2, 2026 03:34
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants