Skip to content

Stop unknown coprocessor stats / no output from shell commands.#1786

Merged
mcm001 merged 1 commit intoPhotonVision:mainfrom
Juniormunk:FixUnknown
Feb 17, 2025
Merged

Stop unknown coprocessor stats / no output from shell commands.#1786
mcm001 merged 1 commit intoPhotonVision:mainfrom
Juniormunk:FixUnknown

Conversation

@Juniormunk
Copy link
Contributor

@Juniormunk Juniormunk commented Feb 17, 2025

Description

Join the threads and wait for them to finish reading the input/error streams before returning.

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 v2024.3.1
  • If this PR addresses a bug, a regression test for it is added

@Juniormunk Juniormunk requested a review from a team as a code owner February 17, 2025 17:03
@mcm001
Copy link
Contributor

mcm001 commented Feb 17, 2025

TY! I think we should probably replace isOutputCompleted and isErrorCompleted with this same join (or set a timeout, as appropriate), or delete these if they're redundant, as well. For example, this already waits for completion, so we don't need that extra wait anymore.

Copy link
Contributor

@mcm001 mcm001 left a comment

Choose a reason for hiding this comment

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

Looks like it'll fix a race

@mcm001 mcm001 merged commit 75c289f into PhotonVision:main Feb 17, 2025
36 checks passed
@Gold856 Gold856 added the backend Things relating to photon-core and photon-server label Aug 4, 2025
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.

3 participants