-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Custom variables #83
Custom variables #83
Conversation
alex-Arc
commented
Sep 30, 2024
•
edited
Loading
edited
- fetch custom fields on referent message from ontime
- add config option to dissable custom fields
- add variables for all custom fields to Now, Next and Prev
- add feedback for custom field value
3254fae
to
d57cc79
Compare
d57cc79
to
dfaecf2
Compare
@cpvalente this is ready |
@@ -4,8 +4,9 @@ export interface OntimeConfig { | |||
host: string | |||
port: string | |||
ssl: boolean | |||
version: string | |||
version: string //TODO: remove |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
??
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it is a config field no longer in use
but still used by the upgrade script, think we can remove it with the next major ontime release
refetchEvents: boolean | ||
customToVariable: boolean |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion: could we find a better name here? this could be difficult to understand for future us. How about convertCustomFieldsToVariable
, it is a bit verbose, but that is what tersers are for?
type: 'checkbox', | ||
default: true, | ||
width: 12, | ||
tooltip: 'Whether Ontime custom fields should be written to variables.', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question: should this not just be the default behaviour?
Why do we want this to be an option? is it to avoid the fetch?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it is default to true
one might want to avoid the potentially large amount of extra variables clogging up the variable list
and it causes a regeneration of all actions and feedbacks
self.setVariableValues({ | ||
[variableId.TitleNow]: val?.title ?? '', | ||
[variableId.NoteNow]: val?.note ?? '', | ||
[variableId.CueNow]: val?.cue ?? '', | ||
[variableId.IdNow]: val?.id ?? '', | ||
[variableId.TitleNow]: val?.title, | ||
[variableId.NoteNow]: val?.note, | ||
[variableId.CueNow]: val?.cue, | ||
[variableId.IdNow]: val?.id, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question: if val
is null
, we will set all of these to undefined
but we know if val
is null
before the operation, could we avoid some work here for those cases??
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if the val is null we need to clear the variable so there is not stale data
setVariableValues
only accepts string | number | boolean | undefined
so i think this is the quickest and most efficient way?
const prefix = self.config.ssl ? 'https' : 'http' | ||
const host = sanitizeHost(self.config.host) | ||
|
||
self.log('debug', 'fetching events from ontime') | ||
try { | ||
const response = await fetch(`${prefix}://${host}:${self.config.port}/data/rundown`, { | ||
method: 'GET', | ||
headers: { Etag: rundownEtag }, | ||
headers: { 'if-none-match': rundownEtag, 'cache-control': '3600', pragma: '' }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice 👍
|
||
export async function fetchCustomFields(self: OnTimeInstance, ontime: OntimeV3): Promise<void> { | ||
//TODO: this might need to be updated on an interval | ||
async function fetchCustomFields(self: OnTimeInstance, ontime: OntimeV3): Promise<boolean> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question: maybe we can make changes in ontime to send a message when the custom fields change? like
refetch: {
changes: 'custom-fields',
}