Skip to content

WIP: feat/show upload hash --> improve typehints around ShowSpecification#18

Merged
ntamas merged 8 commits intomainfrom
feat/show-upload-hash
Jan 26, 2026
Merged

WIP: feat/show upload hash --> improve typehints around ShowSpecification#18
ntamas merged 8 commits intomainfrom
feat/show-upload-hash

Conversation

@vasarhelyi
Copy link
Contributor

@vasarhelyi vasarhelyi commented Jan 23, 2026

This PR parses an optional show hash that is coming from latest Live (https://github.com/skybrush-io/live/tree/feat/show-upload-hash branch) in connection with the new collective RTH support.
It is backwards compatible, if no hash is provided it defaults to None.

We abandoned the new show hash param, but some typehint updates still remained in this PR so can be merged now, it won't alter functionality.

@vasarhelyi vasarhelyi requested a review from ntamas January 23, 2026 14:36
@ntamas
Copy link
Member

ntamas commented Jan 24, 2026

Before we go deeper into the review process: I don't understand a few conceptual things:

What's the purpose of the show hash here?

Is the purpose to be able to distinguish between different upload sessions on the server side? If so, then we don't need a hash because that could be identical for different upload sessions of the same show. We need a "session ID" instead, which can be an arbitrary opaque string generated on Live's side with nanoid or anything else.

Or, is the purpose to be able to distinguish whether the show being uploaded is still the same show as the previous one so we can collect the RTH plan timestamps? In that case, we don't need a hash either -- we need a unique show ID, provided by the design software and most likely already pre-generated in the .skyc file, and the perfect place to embed this would be the show specification itself, no need for a separate argument.

Or, do we want to distinguish between uploads of the same show with different geofence or safety configurations? In that case, the hash can still belong to the show specification as the geofence configuration and other safety settings are also there.

So, all in all, for me it seems that either we don't need a separate argument (and we could just embed the ID in the show specification), or if we do, then we need a session ID, not a hash.

@vasarhelyi vasarhelyi changed the title feat/show upload hash WIP: feat/show upload hash Jan 24, 2026
@vasarhelyi vasarhelyi changed the title WIP: feat/show upload hash WIP: feat/show upload hash --> improve typehints around ShowSpecification Jan 26, 2026
@vasarhelyi
Copy link
Contributor Author

@ntamas I removed the proposed show hash functionality, some typehint improvements remained, please merge.

@ntamas ntamas merged commit 4c31782 into main Jan 26, 2026
3 checks passed
@vasarhelyi vasarhelyi deleted the feat/show-upload-hash branch January 26, 2026 16:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants