Skip to content

Commit c286ba1

Browse files
authored
When a blocks module is loaded, check mrc_call_python_function blocks. (wpilibsuite#159)
* When a blocks module is loaded, check mrc_call_python_function blocks. For component method blocks, check whether the component has been renamed. Update the mrc_call_python_function if necessary. For robot method blocks, check whether the method has been renamed or whether its return value or arguments have changed. Update the mrc_call_python_function if possible. If not possible, add a warning on the block. * Rebuild the FIELD_COMPONENT_NAME field if the component name was changed in the robot. Fixed a problem where if the user changes the component name (by selecting a different one from the dropdown) on a mrc_call_python_function block, it needs to change the block id that it saves in extra state.
1 parent 1374bde commit c286ba1

File tree

8 files changed

+185
-75
lines changed

8 files changed

+185
-75
lines changed

src/blocks/mrc_call_python_function.ts

Lines changed: 164 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,8 @@ export type FunctionArg = {
6363
type: string,
6464
};
6565

66+
const WARNING_ID_FUNCTION_CHANGED = 'function changed';
67+
6668
// Functions used for creating blocks for the toolbox.
6769

6870
export function addModuleFunctionBlocks(
@@ -220,13 +222,12 @@ function createInstanceMethodBlock(
220222
}
221223

222224
export function getInstanceComponentBlocks(
223-
componentType: string,
224-
componentName: string): ToolboxItems.ContentsType[] {
225+
component: CommonStorage.Component): ToolboxItems.ContentsType[] {
225226
const contents: ToolboxItems.ContentsType[] = [];
226227

227-
const classData = getClassData(componentType);
228+
const classData = getClassData(component.className);
228229
if (!classData) {
229-
throw new Error('Could not find classData for ' + componentType);
230+
throw new Error('Could not find classData for ' + component.className);
230231
}
231232
const functions = classData.instanceMethods;
232233

@@ -241,26 +242,27 @@ export function getInstanceComponentBlocks(
241242
if (findSuperFunctionData(functionData, componentFunctions)) {
242243
continue;
243244
}
244-
const block = createInstanceComponentBlock(componentName, functionData);
245+
const block = createInstanceComponentBlock(component, functionData);
245246
contents.push(block);
246247
}
247248

248249
return contents;
249250
}
250251

251252
function createInstanceComponentBlock(
252-
componentName: string, functionData: FunctionData): ToolboxItems.Block {
253+
component: CommonStorage.Component, functionData: FunctionData): ToolboxItems.Block {
253254
const extraState: CallPythonFunctionExtraState = {
254255
functionKind: FunctionKind.INSTANCE_COMPONENT,
255256
returnType: functionData.returnType,
256257
args: [],
257258
tooltip: functionData.tooltip,
258259
importModule: '',
259-
componentClassName: functionData.declaringClassName,
260-
componentName: componentName,
260+
componentClassName: component.className,
261+
componentName: component.name,
262+
componentBlockId: component.blockId,
261263
};
262264
const fields: {[key: string]: any} = {};
263-
fields[FIELD_COMPONENT_NAME] = componentName;
265+
fields[FIELD_COMPONENT_NAME] = component.name;
264266
fields[FIELD_FUNCTION_NAME] = functionData.functionName;
265267
const inputs: {[key: string]: any} = {};
266268
// For INSTANCE_COMPONENT functions, the 0 argument is 'self', but
@@ -307,6 +309,7 @@ function createInstanceRobotBlock(method: CommonStorage.Method): ToolboxItems.Bl
307309
returnType: method.returnType,
308310
actualFunctionName: method.pythonName,
309311
args: [],
312+
classMethodDefBlockId: method.blockId,
310313
};
311314
const fields: {[key: string]: any} = {};
312315
fields[FIELD_FUNCTION_NAME] = method.visibleName;
@@ -337,17 +340,19 @@ function createInstanceRobotBlock(method: CommonStorage.Method): ToolboxItems.Bl
337340

338341
//..............................................................................
339342

340-
export type CallPythonFunctionBlock = Blockly.Block & CallPythonFunctionMixin;
343+
export type CallPythonFunctionBlock = Blockly.Block & CallPythonFunctionMixin & Blockly.BlockSvg;
341344
interface CallPythonFunctionMixin extends CallPythonFunctionMixinType {
342345
mrcFunctionKind: FunctionKind,
343346
mrcReturnType: string,
344347
mrcArgs: FunctionArg[],
345348
mrcTooltip: string,
346349
mrcImportModule: string,
347350
mrcActualFunctionName: string,
348-
mrcExportedFunction: boolean,
351+
mrcClassMethodDefBlockId: string,
349352
mrcComponentClassName: string,
350-
mrcComponentName: string, // Do not access directly. Call getComponentName.
353+
mrcComponents: CommonStorage.Component[],
354+
mrcComponentName: string,
355+
mrcComponentBlockId: string,
351356
renameMethod(this: CallPythonFunctionBlock, newName: string): void;
352357
mutateMethod(this: CallPythonFunctionBlock, defBlockExtraState: ClassMethodDefExtraState): void;
353358
}
@@ -385,9 +390,9 @@ type CallPythonFunctionExtraState = {
385390
*/
386391
actualFunctionName?: string,
387392
/**
388-
* True if this blocks refers to an exported function (for example, from the Robot).
393+
* The id of the mrc_class_method_def type that defines the method. Specified only if the function kind is INSTANCE_ROBOT.
389394
*/
390-
exportedFunction?: boolean,
395+
classMethodDefBlockId?: string,
391396
/**
392397
* The component name. Specified only if the function kind is INSTANCE_COMPONENT.
393398
*/
@@ -396,6 +401,10 @@ type CallPythonFunctionExtraState = {
396401
* The component class name. Specified only if the function kind is INSTANCE_COMPONENT.
397402
*/
398403
componentClassName?: string,
404+
/**
405+
* The id of the mrc_component type that defines the component. Specified only if the function kind is INSTANCE_COMPONENT.
406+
*/
407+
componentBlockId?: string,
399408
};
400409

401410
const CALL_PYTHON_FUNCTION = {
@@ -444,7 +453,7 @@ const CALL_PYTHON_FUNCTION = {
444453
const className = this.mrcComponentClassName;
445454
const functionName = this.getFieldValue(FIELD_FUNCTION_NAME);
446455
tooltip = 'Calls the instance method ' + className + '.' + functionName +
447-
' on the component named ' + this.getComponentName() + '.';
456+
' on the component named ' + this.getFieldValue(FIELD_COMPONENT_NAME) + '.';
448457
break;
449458
}
450459
case FunctionKind.INSTANCE_ROBOT: {
@@ -487,14 +496,20 @@ const CALL_PYTHON_FUNCTION = {
487496
if (this.mrcActualFunctionName) {
488497
extraState.actualFunctionName = this.mrcActualFunctionName;
489498
}
499+
if (this.mrcClassMethodDefBlockId) {
500+
extraState.classMethodDefBlockId = this.mrcClassMethodDefBlockId;
501+
}
490502
if (this.mrcComponentClassName) {
491503
extraState.componentClassName = this.mrcComponentClassName;
492504
}
493505
if (this.getField(FIELD_COMPONENT_NAME)) {
494-
extraState.componentName = this.getComponentName();
495-
}
496-
if (this.mrcExportedFunction) {
497-
extraState.exportedFunction = this.mrcExportedFunction;
506+
extraState.componentName = this.getFieldValue(FIELD_COMPONENT_NAME);
507+
for (const component of this.mrcComponents) {
508+
if (component.name == extraState.componentName) {
509+
extraState.componentBlockId = component.blockId;
510+
break;
511+
}
512+
}
498513
}
499514
return extraState;
500515
},
@@ -519,12 +534,25 @@ const CALL_PYTHON_FUNCTION = {
519534
? extraState.importModule : '';
520535
this.mrcActualFunctionName = extraState.actualFunctionName
521536
? extraState.actualFunctionName : '';
522-
this.mrcExportedFunction = extraState.exportedFunction
523-
? extraState.exportedFunction : false;
537+
this.mrcClassMethodDefBlockId = extraState.classMethodDefBlockId
538+
? extraState.classMethodDefBlockId : '';
524539
this.mrcComponentClassName = extraState.componentClassName
525540
? extraState.componentClassName : '';
541+
// Get the list of components whose type matches this.mrcComponentClassName so we can
542+
// create a dropdown that has the appropriate component name choices.
543+
this.mrcComponents = [];
544+
const editor = Editor.getEditorForBlocklyWorkspace(this.workspace);
545+
if (editor) {
546+
editor.getComponentsFromRobot().forEach(component => {
547+
if (component.className === this.mrcComponentClassName) {
548+
this.mrcComponents.push(component);
549+
}
550+
});
551+
}
526552
this.mrcComponentName = extraState.componentName
527553
? extraState.componentName : '';
554+
this.mrcComponentBlockId = extraState.componentBlockId
555+
? extraState.componentBlockId : '';
528556
this.updateBlock_();
529557
},
530558
/**
@@ -596,25 +624,14 @@ const CALL_PYTHON_FUNCTION = {
596624
break;
597625
}
598626
case FunctionKind.INSTANCE_COMPONENT: {
599-
const componentNames: string[] = [];
600-
// Get the list of component names whose type matches this.mrcComponentClassName so we can
601-
// create a dropdown that has the appropriate component names.
602-
const editor = Editor.getEditorForBlocklyWorkspace(this.workspace);
603-
if (editor) {
604-
const components = editor.getComponentsFromRobot();
605-
components.forEach(component => {
606-
if (component.className === this.mrcComponentClassName) {
607-
componentNames.push(component.name);
608-
}
609-
});
610-
}
611-
const componentName = this.getComponentName();
612-
if (!componentNames.includes(componentName)) {
613-
componentNames.push(componentName);
627+
const componentNameChoices = [];
628+
this.mrcComponents.forEach(component => componentNameChoices.push(component.name));
629+
if (!componentNameChoices.includes(this.mrcComponentName)) {
630+
componentNameChoices.push(this.mrcComponentName);
614631
}
615632
this.appendDummyInput('TITLE')
616633
.appendField('call')
617-
.appendField(createFieldDropdown(componentNames), FIELD_COMPONENT_NAME)
634+
.appendField(createFieldDropdown(componentNameChoices), FIELD_COMPONENT_NAME)
618635
.appendField('.')
619636
.appendField(createFieldNonEditableText(''), FIELD_FUNCTION_NAME);
620637
break;
@@ -660,15 +677,6 @@ const CALL_PYTHON_FUNCTION = {
660677
this.removeInput('ARG' + i);
661678
}
662679
},
663-
getComponentName(this: CallPythonFunctionBlock): string {
664-
// If the COMPONENT_NAME field has been created, get the field value, which the user may have changed.
665-
// If the COMPONENT_NAME field has not been created, get the component name from this.mrcComponentName.
666-
return (this.getField(FIELD_COMPONENT_NAME))
667-
? this.getFieldValue(FIELD_COMPONENT_NAME) : this.mrcComponentName;
668-
},
669-
getComponentClassName(this: CallPythonFunctionBlock): string {
670-
return this.mrcComponentClassName;
671-
},
672680
renameMethod: function(this: CallPythonFunctionBlock, newName: string): void {
673681
this.setFieldValue(newName, FIELD_FUNCTION_NAME);
674682
},
@@ -686,6 +694,115 @@ const CALL_PYTHON_FUNCTION = {
686694
});
687695
this.updateBlock_();
688696
},
697+
onLoad: function(this: CallPythonFunctionBlock): void {
698+
const warnings: string[] = [];
699+
700+
// If this block is calling a component method, check that the component
701+
// still exists and hasn't been changed.
702+
// If the component doesn't exist, put a visible warning on this block.
703+
// If the component has changed, update the block if possible or put a
704+
// visible warning on it.
705+
if (this.mrcFunctionKind === FunctionKind.INSTANCE_COMPONENT) {
706+
let foundComponent = false;
707+
for (const component of this.mrcComponents) {
708+
if (component.blockId === this.mrcComponentBlockId) {
709+
foundComponent = true;
710+
711+
// If the component name has changed, we can handle that.
712+
if (this.getFieldValue(FIELD_COMPONENT_NAME) !== component.name) {
713+
// Replace the FIELD_COMPONENT_NAME field.
714+
const titleInput = this.getInput('TITLE')
715+
if (titleInput) {
716+
let indexOfComponentName = -1;
717+
for (let i = 0, field; (field = titleInput.fieldRow[i]); i++) {
718+
if (field.name === FIELD_COMPONENT_NAME) {
719+
indexOfComponentName = i;
720+
break;
721+
}
722+
}
723+
if (indexOfComponentName != -1) {
724+
const componentNameChoices = [];
725+
this.mrcComponents.forEach(component => componentNameChoices.push(component.name));
726+
titleInput.removeField(FIELD_COMPONENT_NAME);
727+
titleInput.insertFieldAt(indexOfComponentName,
728+
createFieldDropdown(componentNameChoices), FIELD_COMPONENT_NAME);
729+
}
730+
this.setFieldValue(component.name, FIELD_COMPONENT_NAME);
731+
}
732+
}
733+
734+
// Since we found the component, we can break out of the loop.
735+
break;
736+
}
737+
}
738+
if (!foundComponent) {
739+
warnings.push('This block calls a method on a component that no longer exists.');
740+
}
741+
742+
// TODO(lizlooney): Could the component's method have change?
743+
}
744+
745+
// If this block is calling a robot method, check that the robot method
746+
// still exists and hasn't been changed.
747+
// If the robot method doesn't exist, put a visible warning on this block.
748+
// If the robot method has changed, update the block if possible or put a
749+
// visible warning on it.
750+
if (this.mrcFunctionKind === FunctionKind.INSTANCE_ROBOT) {
751+
let foundRobotMethod = false;
752+
const editor = Editor.getEditorForBlocklyWorkspace(this.workspace);
753+
if (editor) {
754+
const robotMethods = editor.getMethodsFromRobot();
755+
for (const robotMethod of robotMethods) {
756+
if (robotMethod.blockId === this.mrcClassMethodDefBlockId) {
757+
foundRobotMethod = true;
758+
759+
// If the function name has changed, we can fix this block.
760+
if (this.mrcActualFunctionName !== robotMethod.pythonName) {
761+
this.mrcActualFunctionName = robotMethod.pythonName;
762+
}
763+
if (this.getFieldValue(FIELD_FUNCTION_NAME) !== robotMethod.visibleName) {
764+
this.setFieldValue(robotMethod.visibleName, FIELD_FUNCTION_NAME);
765+
}
766+
767+
// Other things are more difficult.
768+
if (this.mrcReturnType !== robotMethod.returnType) {
769+
warnings.push('This block calls a method whose return type has changed.');
770+
}
771+
if (this.mrcArgs.length !== robotMethod.args.length - 1) {
772+
warnings.push('This block calls a method whose arguments have changed.');
773+
} else {
774+
for (let i = 1; i < robotMethod.args.length; i++) { // Skip the self argument.
775+
if (this.mrcArgs[i-1].name != robotMethod.args[i].name) {
776+
warnings.push('This block calls a method whose arguments have changed.');
777+
break;
778+
}
779+
if (this.mrcArgs[i-1].type != robotMethod.args[i].type) {
780+
warnings.push('This block calls a method whose arguments have changed.');
781+
break;
782+
}
783+
}
784+
}
785+
786+
// Since we found the robot method, we can break out of the loop.
787+
break;
788+
}
789+
}
790+
if (!foundRobotMethod) {
791+
warnings.push('This block calls a method that no longer exists.');
792+
}
793+
}
794+
}
795+
796+
if (warnings.length) {
797+
// Add a warnings to the block.
798+
const warningText = warnings.join('\n\n');
799+
this.setWarningText(warningText, WARNING_ID_FUNCTION_CHANGED);
800+
this.bringToFront();
801+
} else {
802+
// Clear the existing warning on the block.
803+
this.setWarningText(null, WARNING_ID_FUNCTION_CHANGED);
804+
}
805+
},
689806
};
690807

691808
export const setup = function() {
@@ -748,7 +865,7 @@ export const pythonFromBlock = function(
748865
break;
749866
}
750867
case FunctionKind.INSTANCE_COMPONENT: {
751-
const componentName = callPythonFunctionBlock.getComponentName();
868+
const componentName = block.getFieldValue(FIELD_COMPONENT_NAME);
752869
const functionName = callPythonFunctionBlock.mrcActualFunctionName
753870
? callPythonFunctionBlock.mrcActualFunctionName
754871
: block.getFieldValue(FIELD_FUNCTION_NAME);

src/blocks/mrc_class_method_def.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -404,6 +404,7 @@ export const pythonFromBlock = function (
404404
if (block.mrcCanBeCalledOutsideClass) {
405405
// Update the mrcMethod.
406406
block.mrcMethod = {
407+
blockId: block.id,
407408
visibleName: block.getFieldValue('NAME'),
408409
pythonName: funcName,
409410
returnType: block.mrcReturnType,

src/blocks/mrc_get_python_variable.ts

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,6 @@ interface GetPythonVariableMixin extends GetPythonVariableMixinType {
191191
mrcSelfType: string,
192192
mrcImportModule: string,
193193
mrcActualVariableName: string,
194-
mrcExportedVariable: boolean,
195194
mrcKey: string,
196195
}
197196
type GetPythonVariableMixinType = typeof GET_PYTHON_VARIABLE;
@@ -227,10 +226,6 @@ type GetPythonVariableExtraState = {
227226
* the FIELD_VARIABLE_NAME field.
228227
*/
229228
actualVariableName?: string,
230-
/**
231-
* True if this blocks refers to an exported variable (for example, from the Robot).
232-
*/
233-
exportedVariable?: boolean,
234229
};
235230

236231
const GET_PYTHON_VARIABLE = {
@@ -289,7 +284,6 @@ const GET_PYTHON_VARIABLE = {
289284
const extraState: GetPythonVariableExtraState = {
290285
varKind: this.mrcVarKind,
291286
moduleOrClassName: this.mrcModuleOrClassName,
292-
exportedVariable: this.mrcExportedVariable,
293287
};
294288
if (this.mrcVarType) {
295289
extraState.varType = this.mrcVarType;
@@ -324,8 +318,6 @@ const GET_PYTHON_VARIABLE = {
324318
? extraState.importModule : '';
325319
this.mrcActualVariableName = extraState.actualVariableName
326320
? extraState.actualVariableName : '';
327-
this.mrcExportedVariable = extraState.exportedVariable
328-
? extraState.exportedVariable : false;
329321
this.mrcKey = createKey(this.mrcVarKind, this.mrcModuleOrClassName, this.mrcVarType);
330322
this.updateBlock_();
331323
},

src/blocks/mrc_mechanism_component_holder.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,7 @@ const MECHANISM_COMPONENT_HOLDER = {
144144

145145
if (componentName && componentType) {
146146
components.push({
147+
blockId: componentBlock.id,
147148
name: componentName,
148149
className: componentType,
149150
});

0 commit comments

Comments
 (0)