Skip to content

Conversation

@stephen99scott
Copy link
Member

Three main edits:

  1. Added isSubsystem.m

    • Checks if a block is a subsystem
    • Used in determining whether to display 'Convert Subsystem' in the Simulink context menu
  2. Added subToSimFcn.m

    • Converts subsystem to Sim-fcn by adding a trigger port and swapping ports for arguments
  3. Edited sl_customization.m

    • Added function callbacks for subToSimFcn

Copy link
Member

@monikajaskolka monikajaskolka left a comment

Choose a reason for hiding this comment

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

sl_customization.m

  • Line 40:
  • Line 33, 47: Capitalize "To" to be consistent with the other menus.
  • Check that a new Simulink Function with the user's input is allowed to be added to the model without causing simulations errors, e.g., if a function of that name already exists in scope. Use getCallableFunctions.m to get a list of all Simulink Functions in scope at the model level where the new function is to be added, and if the new function has the same name (including qualifier) as another that is in scope (regardless of parameter list), then it cannot be added.

isSubsystem.m

Even though this function is used in the specific sense of converting a subsystem, it should be made more general, so we can reuse it in the future. In particular on Line 11, don't assume that the input will be a cell array with a pathname. When dealing with inputs that can represent a block or other model elements, you should allow the function to accept it as a pathname or handle. So, the input can be a char array, a numeric block handle, a cell array of names, or a vector of handles. The function fails for these cases:

  • isSubsystem({}), isSubsystem('') Empty input
  • isSubsystem(gcb) Char array name
  • isSubsystem(gcbh) Numeric handle
  • isSubsystem({gcb, gcb}), isSubsystem([gcbh, gcbh]) Multiple elements. This should return [1, 1]. Note: The function get_param already works on multiple elements.

Other

Please make the code formatting consistent. In particular, add spaces around assignment (=), and one space after commas. You can try using MBeautifier to automatically do it, but I haven't used it myself. Unfortunately MathWorks doesn't provide code beautification other than automatic code indentation.

@monikajaskolka
Copy link
Member

  1. I noticed that when the inports and outports of a subsystem have invalid variable names (e.g. they contain spaces) the resulting argin and argout names are a "arg1", "arg2", etc. for both the argin and argout. If we are giving them generic names automatically, they should be "ArgIn1" , "ArgIn2", etc. for inports and "ArgOut1", "ArgOut2", etc. for outports. But that leads me to point Converting Subsystem to Simulink Function from inside existing Simulink Function #2...

Before:


before

After:


after

  1. Can you use the genvarname command to take the existing block names and use them as the argument names? Could this also be done to use the subsystem name as the function name?

%% Add Trigger to Subsystem

% Disable subsystem link
set_param(subsystem, 'LinkStatus', 'inactive');
Copy link
Member

Choose a reason for hiding this comment

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

Can you check if the LinkStatus is okay to leave as 'inactive', or do we want to break it entirely by setting it to 'none'? See this for more info: https://www.mathworks.com/help/simulink/ug/control-linked-block-status-programmatically.html

set_param(subsystem, 'LinkStatus', 'inactive');

% Adding the trigger block to the subsystem and calibrating its parameters
TriggerPath = append(subsystem, '/', simulinkFcnName);
Copy link
Member

Choose a reason for hiding this comment

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

Variable names should start with lowercase.

try
assert(not(strcmp(parameters{port, 4}, 'Inherit: auto')));
catch
disp(['Invalid data type of port', newline, ports{port}, newline,...
Copy link
Member

Choose a reason for hiding this comment

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

Can this be printed on one line? Please also add single quotes around 'Inherit' and 'double'. To print a single quite you escape it with another single quote.

try
assert(not(strcmp(parameters{port, 6}, '-1')))
catch
disp(['Invalid port dimensions of port:', newline, ports{port},...
Copy link
Member

Choose a reason for hiding this comment

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

Print on one line please.

subToSimFcn(subsystem{1}, simulinkFcnName, 'scoped');
end

function schema=toGlobalSimFcn(callbackInfo)
Copy link
Member

Choose a reason for hiding this comment

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

Spaces around = please.

schema.ChildrenFcns = {@toScopedSimFcn, @toGlobalSimFcn};
end

function schema=toScopedSimFcn(callbackInfo)
Copy link
Member

Choose a reason for hiding this comment

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

Spaces around = please.

% Check each block to see if its a subsystem
for block = 1:length(blocks)
try
b(block) = strcmpi(get_param(blocks(block),'BlockType'),'SubSystem');
Copy link
Member

Choose a reason for hiding this comment

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

Spaces around commas please.

@@ -0,0 +1,37 @@
function goodName = isGoodSimFcnName(subsystem, simulinkFcnName)
% isGoodSimFcnName Checks to see if a Simulink-fcn name clashes with
Copy link
Member

Choose a reason for hiding this comment

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

Please be precise and consistent with language in the comments. Please write out 'Simulink Function' instead of 'Simulink-fcn' or 'Simulink-function'. For actual parameter/variable/function names it's okay to shorten it, but be aware that the Simulink language has another block called 'Fcn'.

@monikajaskolka monikajaskolka merged commit e59dbd0 into master Jun 24, 2020
@monikajaskolka monikajaskolka deleted the subsys-to-simfunc branch June 24, 2020 20:15
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.

3 participants