Skip to content
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

[blockly] Adjust persistence blocks to breaking changes #2564

Merged
merged 4 commits into from
May 7, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,18 @@ export default function defineOHBlocks_Persistence (f7, isGraalJs, persistenceSe
this.appendDummyInput()
.appendField('get the')
.appendField(new Blockly.FieldDropdown([
['state average', 'averageSince'], ['state delta', 'deltaSince'],
['state deviation', 'deviationSince'], ['state variance', 'varianceSince'], ['evolution rate', 'evolutionRateSince'],
['state minimum', 'minimumSince'], ['state maximum', 'maximumSince'], ['state sum', 'sumSince'],
['previous state value', 'previousState'], ['previous state numeric value', 'previousNumericState'], ['previous state value time', 'previousStateTime'],
['historic state', 'historicState']
['persisted state', 'persistedState'],
['historic state average', 'averageSince'], ['future state average', 'averageUntil'],
['historic state delta', 'deltaSince'], ['future state delta', 'deltaUntil'],
['historic state deviation', 'deviationSince'], ['future state deviation', 'deviationUntil'],
['historic state variance', 'varianceSince'], ['future state variance', 'varianceUntil'],
['historic evolution rate', 'evolutionRateSince'], ['future evolution rate', 'evolutionRateUntil'],
['historic state minimum', 'minimumSince'], ['future state minimum', 'minimumUntil'],
['historic state maximum', 'maximumSince'], ['future state maximum', 'maximumUntil'],
['historic state sum', 'sumSince'], ['future state sum', 'sumUntil'],
['previous state value', 'previousState'], ['next state value', 'nextState'],
['previous state numeric value', 'previousNumericState'], ['next state numeric value', 'nextNumericState'],
['previous state value time', 'previousStateTime'], ['next state value time', 'nextStateTime']
], this.handleTypeSelection.bind(this)
), 'methodName')
this.methodName = this.getFieldValue('methodName')
Expand All @@ -35,23 +42,33 @@ export default function defineOHBlocks_Persistence (f7, isGraalJs, persistenceSe
this.setInputsInline(false)
this.setOutput(true, null)
this.setColour(0)
let thisBlock = this

this.setTooltip(function () {
let methodName = thisBlock.getFieldValue('methodName')
this.setTooltip(() => {
let methodName = this.getFieldValue('methodName')
let TIP = {
'averageSince': 'Gets the average value of the State of a persisted Item since a certain point in time. This method uses a time-weighted average calculation',
'averageUntil': 'Gets the average value of the State of a persisted Item until a certain point in time. This method uses a time-weighted average calculation',
'deltaSince': 'Gets the difference in value of the State of a given Item since a certain point in time',
'deltaUntil': 'Gets the difference in value of the State of a given Item until a certain point in time',
'deviationSince': 'Gets the standard deviation of the state of the given Item since a certain point in time',
'deviationUntil': 'Gets the standard deviation of the state of the given Item until a certain point in time',
'varianceSince': 'Gets the variance of the state of the given Item since a certain point in time',
'varianceUntil': 'Gets the variance of the state of the given Item until a certain point in time',
'evolutionRateSince': 'Gets the evolution rate of the state of a given Item since a certain point in time',
'evolutionRateUntil': 'Gets the evolution rate of the state of a given Item until a certain point in time',
'minimumSince': 'Gets the minimum value of the State of a persisted Item since a certain point in time',
'minimumUntil': 'Gets the minimum value of the State of a persisted Item until a certain point in time',
'maximumSince': 'Gets the maximum value of the State of a persisted Item since a certain point in time',
'maximumUntil': 'Gets the maximum value of the State of a persisted Item until a certain point in time',
'sumSince': 'Gets the sum of the previous States of a persisted Item since a certain point in time',
'sumUntil': 'Gets the sum of the previous States of a persisted Item until a certain point in time',
'previousState': 'Gets the previous state with option to skip to different value as current',
'nextState': 'Gets the next state with option to skip to different value as current',
'previousNumericState': 'Gets the previous state without the unit with option to skip to different value as current',
'nextNumericState': 'Gets the next state without the unit with option to skip to different value as current',
'previousStateTime': 'Gets the time when previous state last occurred with option to skip to different value as current',
'historicState': 'Gets the historic state at a certain point in time'
'nextStateTime': 'Gets the time when next state will occur with option to skip to different value as current',
'persisted': 'Gets the persisted state at a certain point in time'
}
return TIP[methodName]
})
Expand All @@ -68,7 +85,7 @@ export default function defineOHBlocks_Persistence (f7, isGraalJs, persistenceSe
if (!persistenceNameInput.getShadowDom()) {
persistenceNameInput.setShadowDom(Blockly.utils.xml.textToDom('<shadow type="oh_persistence_dropdown" />'))
}
if (this.methodName === 'previousState' || this.methodName === 'previousNumericState' || this.methodName === 'previousStateTime') {
if (['previousState', 'nextState', 'previousNumericState', 'nextNumericState', 'previousStateTime', 'nextStateTime'].includes(this.methodName)) {
if (this.getInput('dayInfo')) {
this.removeInput('dayInfo')
}
Expand All @@ -83,7 +100,7 @@ export default function defineOHBlocks_Persistence (f7, isGraalJs, persistenceSe
this.removeInput('skipPrevious')
}

const preposition = (this.methodName === 'historicState') ? 'at' : 'since'
const preposition = (this.methodName === 'persistedState') ? 'at' : (this.methodName.endsWith('Until') ? 'until' : 'since')

if (!this.getInput('dayInfo')) {
this.appendValueInput('dayInfo')
Expand Down Expand Up @@ -121,28 +138,53 @@ export default function defineOHBlocks_Persistence (f7, isGraalJs, persistenceSe
const persistenceExtension = (persistenceName === '\'default\'') ? '' : `, ${persistenceName}`

switch (methodName) {
// Returning JS PersistedItem (GraalJS) or org.openhab.core.persistence.HistoricItem
case 'maximumSince':
case 'maximumUntil':
case 'minimumSince':
case 'historicState':
case 'minimumUntil':
case 'persistedState':
dayInfo = javascriptGenerator.valueToCode(block, 'dayInfo', javascriptGenerator.ORDER_NONE)
code = (isGraalJs) ? `${itemCode}.history.${methodName}(${dayInfo}${persistenceExtension})?.state` : `${persistence}.${methodName}(${itemCode}, ${dayInfo}${persistenceExtension}).getState()`
code = (isGraalJs) ? `${itemCode}.persistence.${methodName}(${dayInfo}${persistenceExtension})?.state` : `${persistence}.${methodName}(${itemCode}, ${dayInfo}${persistenceExtension}).getState()`
break

case 'previousState':
code = (isGraalJs) ? `${itemCode}.history.previousState(${skipPrevious}${persistenceExtension})?.state` : `${persistence}.previousState(${itemCode},${skipPrevious}${persistenceExtension}).getState()`
case 'nextState':
code = (isGraalJs) ? `${itemCode}.persistence.${methodName}(${skipPrevious}${persistenceExtension})?.state` : `${persistence}.${methodName}(${itemCode},${skipPrevious}${persistenceExtension}).getState()`
break

case 'previousNumericState':
code = (isGraalJs) ? `${itemCode}.history.previousState(${skipPrevious}${persistenceExtension})?.numericState` : `${persistence}.previousState(${itemCode},${skipPrevious}${persistenceExtension}).getNumericState()`
code = (isGraalJs) ? `${itemCode}.persistence.previousState(${skipPrevious}${persistenceExtension})?.numericState` : `${persistence}.previousState(${itemCode},${skipPrevious}${persistenceExtension}).getNumericState()`
break
case 'nextNumericState':
code = (isGraalJs) ? `${itemCode}.persistence.nextState(${skipPrevious}${persistenceExtension})?.numericState` : `${persistence}.nextState(${itemCode},${skipPrevious}${persistenceExtension}).getNumericState()`
break

case 'previousStateTime':
code = (isGraalJs) ? `${itemCode}.history.previousState(${skipPrevious}${persistenceExtension})?.timestamp` : `${persistence}.previousState(${itemCode},${skipPrevious}${persistenceExtension}).getTimestamp()`
code = (isGraalJs) ? `${itemCode}.persistence.previousState(${skipPrevious}${persistenceExtension})?.timestamp` : `${persistence}.previousState(${itemCode},${skipPrevious}${persistenceExtension}).getTimestamp()`
break
case 'nextStateTime':
code = (isGraalJs) ? `${itemCode}.persistence.nextState(${skipPrevious}${persistenceExtension})?.timestamp` : `${persistence}.nextState(${itemCode},${skipPrevious}${persistenceExtension}).getTimestamp()`
break

// Returning JS PersistedState (GraalJS) or org.openhab.core.types.State
case 'averageSince':
case 'averageUntil':
case 'deltaSince':
case 'deltaUntil':
case 'deviationSince':
case 'deviationUntil':
case 'sumSince':
case 'sumUntil':
case 'varianceSince':
case 'varianceUntil':
dayInfo = javascriptGenerator.valueToCode(block, 'dayInfo', javascriptGenerator.ORDER_NONE)
code = (isGraalJs) ? `${itemCode}.persistence.${methodName}(${dayInfo}${persistenceExtension})?.numericState` : `parseFloat(${persistence}.${methodName}(${itemCode}, ${dayInfo}${persistenceExtension}).getState())`
Copy link
Contributor

Choose a reason for hiding this comment

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

@stefan-hoehn This is inconsistent. For GraalJS, you focus on keeping backward compatibility by returning numericState. For Nashorn, it returns the full state.
Moreover, by returning numericState in GraalJS, you basically strip the unit and make it impossible to do straight quantity calculations on the result. I understand this would be a breaking change to remove this, but I think we should do it now rather than wait until after 4.2.
As an alternative, one could create extra methods (something like deltaStateSince). That would allow to keep backward compatibility for GraalJS at the expense of having different and less intuitive names in Blockly vs pure JS. I would prefer the breaking change here.
@florian-h05 What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is not inconsistent, for Nashorn there is also a numeric state returned (note the parseFloat around the state).

I have done this PR this way because after the breaking changes were done to code and the helper library, we needed to update Blockly quickly before the new milestone so it is not effectively „broken“.
Adjusting Blockly to allow use the new functionality was postponed to a later PR.

See #2595 (comment) for my implementation proposal.

break

default:
dayInfo = javascriptGenerator.valueToCode(block, 'dayInfo', javascriptGenerator.ORDER_NONE)
code = (isGraalJs) ? `${itemCode}.history.${methodName}(${dayInfo}${persistenceExtension})` : `${persistence}.${methodName}(${itemCode}, ${dayInfo}${persistenceExtension})`
code = (isGraalJs) ? `${itemCode}.persistence.${methodName}(${dayInfo}${persistenceExtension})` : `${persistence}.${methodName}(${itemCode}, ${dayInfo}${persistenceExtension})`
break
}

Expand All @@ -166,7 +208,7 @@ export default function defineOHBlocks_Persistence (f7, isGraalJs, persistenceSe
persistenceNameInput.setShadowDom(Blockly.utils.xml.textToDom('<shadow type="oh_persistence_dropdown" />'))
}
this.appendValueInput('dayInfo')
.appendField(new Blockly.FieldDropdown([['has changed since', 'changedSince'], ['has been updated since', 'updatedSince']]), 'methodName')
.appendField(new Blockly.FieldDropdown([['has changed since', 'changedSince'], ['will have changed until', 'changedUntil'], ['has been updated since', 'updatedSince'], ['will have been updated until', 'updatedUntil']]), 'methodName')
.setAlign(Blockly.ALIGN_RIGHT)
.setCheck(['ZonedDateTime'])

Expand All @@ -179,7 +221,9 @@ export default function defineOHBlocks_Persistence (f7, isGraalJs, persistenceSe
let methodName = thisBlock.getFieldValue('methodName')
let TIP = {
'changedSince': 'Checks if the State of the Item has (ever) changed since a certain point in time',
'updatedSince': 'Checks if the State of the Item has been updated since a certain point in time'
'changedUntil': 'Checks if the State of the Item will have (ever) changed until a certain point in time',
'updatedSince': 'Checks if the State of the Item has been updated since a certain point in time',
'updatedUntil': 'Checks if the State of the Item will have been updated until a certain point in time'
}
return TIP[methodName]
})
Expand All @@ -194,19 +238,17 @@ export default function defineOHBlocks_Persistence (f7, isGraalJs, persistenceSe
*/
javascriptGenerator.forBlock['oh_persist_changed'] = function (block) {
const itemName = javascriptGenerator.valueToCode(block, 'itemName', javascriptGenerator.ORDER_ATOMIC)

const inputType = blockGetCheckedInputType(block, 'itemName')

let itemCode = generateItemCode(itemName, inputType)

const methodName = block.getFieldValue('methodName')
const dayInfo = javascriptGenerator.valueToCode(block, 'dayInfo', javascriptGenerator.ORDER_NONE)

const persistenceName = javascriptGenerator.valueToCode(block, 'persistenceName', javascriptGenerator.ORDER_NONE)
const persistenceExtension = (persistenceName === '\'default\'') ? '' : `, ${persistenceName}`

let itemCode = generateItemCode(itemName, inputType)

if (isGraalJs) {
return [`${itemCode}.history.${methodName}(${dayInfo}${persistenceExtension})`, javascriptGenerator.ORDER_NONE]
return [`${itemCode}.persistence.${methodName}(${dayInfo}${persistenceExtension})`, javascriptGenerator.ORDER_NONE]
} else {
const { dtf, zdt, getZonedDatetime } = addDateSupport()
const persistence = addPersistence()
Expand All @@ -221,7 +263,11 @@ export default function defineOHBlocks_Persistence (f7, isGraalJs, persistenceSe
Blockly.Blocks['oh_get_persistence_lastupdate'] = {
init: function () {
this.appendDummyInput()
.appendField('last updated date of')
.appendField(new Blockly.FieldDropdown([
['last', 'lastUpdate'], ['next', 'nextUpdate']
]), 'methodName')
this.appendDummyInput()
.appendField(' updated date of')
this.appendValueInput('itemName')
.setCheck(['String', 'oh_item', 'oh_itemtype'])
const persistenceNameInput = this.appendValueInput('persistenceName')
Expand All @@ -234,8 +280,16 @@ export default function defineOHBlocks_Persistence (f7, isGraalJs, persistenceSe
this.setInputsInline(true)
this.setOutput(true, 'ZonedDateTime')
this.setColour(0)
this.setTooltip('Get the last update time of the provided item')
this.setHelpUrl('https://www.openhab.org/docs/configuration/blockly/rules-blockly-persistence.html#provide-last-updated-date-of-an-item')

this.setTooltip(() => {
const methodName = this.getFieldValue('methodName')
const TIP = {
'lastUpdate': 'Get the last update time of the provided item',
'nextUpdate': 'Get the next update time of the provided item'
}
return TIP[methodName]
})
}
}

Expand All @@ -244,6 +298,7 @@ export default function defineOHBlocks_Persistence (f7, isGraalJs, persistenceSe
* Code part
*/
javascriptGenerator.forBlock['oh_get_persistence_lastupdate'] = function (block) {
const methodName = block.getFieldValue('methodName')
const itemName = javascriptGenerator.valueToCode(block, 'itemName', javascriptGenerator.ORDER_ATOMIC)
const inputType = blockGetCheckedInputType(block, 'itemName')
const persistenceName = javascriptGenerator.valueToCode(block, 'persistenceName', javascriptGenerator.ORDER_NONE)
Expand All @@ -252,11 +307,11 @@ export default function defineOHBlocks_Persistence (f7, isGraalJs, persistenceSe
let itemCode = generateItemCode(itemName, inputType)

if (isGraalJs) {
return [`${itemCode}.history.lastUpdate(${persistenceExtension})`, 0]
return [`${itemCode}.persistence.${methodName}(${persistenceExtension})`, 0]
} else {
const { dtf, zdt, getZonedDatetime } = addDateSupport()
const persistence = addPersistence()
let code = `${persistence}.lastUpdate(${itemCode}${persistenceExtension})`
let code = `${persistence}.${methodName}(${itemCode}${persistenceExtension})`
return [code, 0]
}
}
Expand Down
Loading