FMIKit-Simulink icon indicating copy to clipboard operation
FMIKit-Simulink copied to clipboard

Usability Improvements

Open Marvin-TMG opened this issue 4 years ago • 7 comments

To improve the handling when switching FMUs and to reduce the amount of paths that need to be defined when generating code, I would suggest the following two improvements:

  • Store the generated S-function source code and the mex file in the FMU unzip directory as well. This will then allow FMIKit to dispose a single folder when switching FMUs. The only issue I see here is that Matlab may lock the mex file, so when attempting to delete the folder a callback to Matlab is necessary to first clear any existing mex files in the unzip directory.

  • Include the all.c file or FMU source code files directly in the auto generated S-function. I could not get this to work, but if possible, this will avoid the need to specifically specify the path to all.c in the build process.

Marvin-TMG avatar Sep 21 '20 14:09 Marvin-TMG

Can you provide steps to reproduce the problem?

t-sommer avatar Sep 21 '20 15:09 t-sommer

There are basically two seperate issues when switching models.

Issue 1: When loading a different FMU with a different model indentifier and building this s-function from source, the old s-function and source code are not cleaned up, but left behind in the root. Frequent switching will accumulate a lot of s-functions and source codes. Solution: Put the source code and mex function in the unzip directory, FMIKit can then automatically dispose this folder, containing the old S-function and source. Downside: The unzip directory needs to be added to the Matlab path. Perhaps there is a way to use relative paths, but I did not manage to get this to work.

Issue 2: Everytime a different model is loaded new paths to the source folder and the all.c are added to the Simulink build options. This can cause incorrect results and undefined behaviour when changing the unzip directory or when deleting the block and creating a new unzip directory. Solution for Simulink-builds: Instead of adding the paths to the Simulink build options, resolve the paths in the applyDialog when performing the mex call instead. This will avoid accumulation of outdated source paths.

Basically, it all comes down to properly disposing old source code and references to old source code. Also, when a block is deleted, it is probably a good idea to clean up as well.

Marvin-TMG avatar Sep 21 '20 15:09 Marvin-TMG

For issue 2 a possible, somewhat dirty solution is:

function setSFunctionParameters(block)
% Internal API - do not use

userData = getUserData(block);
if isempty(userData)
    return
end

set_param(block, 'FunctionName', userData.functionName, 'Parameters', userData.parameters)

mdl = getfullname(bdroot(block));
[mdldir, ~, ~] = fileparts(which(mdl));
unzipdir = fullfile(mdldir, userData.unzipDirectory);
% addpath(unzipdir);
end


function applyDialog(dialog)

%#ok<*AGROW>

% set the user data
userData = userDataToStruct(dialog.getUserData());
set_param(dialog.blockHandle, 'UserData', userData, 'UserDataPersistent', 'on');

% set the S-function parameters
FMIKit.setSFunctionParameters(dialog.blockHandle)

if userData.useSourceCode

    % generate the S-function source
    dialog.generateSourceSFunction();

    model_identifier = char(dialog.getModelIdentifier());
    unzipdir = char(dialog.getUnzipDirectory());

    pkginfo = what('FMIKit');
    [fmikitdir, ~, ~] = fileparts(pkginfo(1).path);
    
    sfunsrc = ['sfun_' model_identifier '.c' ];

    movefile(sfunsrc, fullfile(unzipdir, sfunsrc));
    
    % build the S-function
    clear(['sfun_' model_identifier])

    disp(['Compiling S-function sfun_' model_identifier])

    mex_args = {};
    
    % debug build
    % mex_args{end+1} = '-g';
    
    % custom inlcude directories
    include_dirs = get_param(gcs, 'SimUserIncludeDirs');
    include_dirs = split_paths(include_dirs);
    for i = 1:numel(include_dirs)
        mex_args{end+1} = ['-I"' include_dirs{i} '"'];
    end
    mex_args{end+1} = ['-I"' fullfile(fmikitdir, 'include') '"'];
    for i = 1:numel(include_dirs)
     	mex_args{end+1} = ['-I"' fullfile(unzipdir, 'sources') '"'];
    end
    
    % shlwapi.lib for Dymola FMUs
    if ispc
        mex_args{end+1} = '-lshlwapi';
    end

    % custom libraries
    libraries = get_param(gcs, 'SimUserLibraries');
    libraries = split_paths(libraries);
    for i = 1:numel(libraries)
        [library_path, library_name, ~] = fileparts(libraries{i});
        mex_args{end+1} = ['-L"' library_path '"'];
        mex_args{end+1} = ['-l' library_name];
    end

    % S-function source
    mex_args{end+1} = fullfile(unzipdir, ['sfun_' model_identifier '.c']);

    % FMU sources
    it = dialog.getSourceFiles().listIterator();
    while it.hasNext()
        mex_args{end+1} = ['"' fullfile(unzipdir, 'sources', it.next()) '"'];
%         mex_args{end+1} = ['"' fullfile('sources', it.next()) '"'];
    end

    for i = 1 : length(mex_args)
        mex_args{i} = strrep(mex_args{i}, newline, '');
    end
    
%     curpath = pwd;
%     cd(fullfile(unzipdir))
    
    try
        mex(mex_args{:})
    catch e
        disp('Failed to compile S-function')
        disp(e.message)
        % rethrow(e)
    end

%     cd(curpath);
end

end

Marvin-TMG avatar Sep 21 '20 15:09 Marvin-TMG

@Marvin-TMG, can you give 32ee19f5093bcc3a931e3e3d739c788b4dc83565 a spin?

t-sommer avatar Oct 05 '20 09:10 t-sommer

Thanks! I will try it once back from holidays next week.

Can you confirm if the following points have been addressed as well, or are planned:

  • Remove/rename the unzip dir and S-function (name) when changing the unzip directory name in the block GUI
  • Remove and regenerate the S-Function when loading a different FMU (and of course regenerate the unzip dir)
  • Remove outdated include directories and source file references from the build options

Perhaps an easy way is to extend the function you created and check if the files and paths are still valid, if not remove these from the build options.

Thanks!

MarvinSt avatar Oct 13 '20 19:10 MarvinSt

Hi Torsten,

The commit is not working for me, the callback is never triggered. Can you confirm that the callback of the simulink block is properly assinged? The simulink block from the commit is pointing for me to closeBlockDialog (for both destroy and delete).

This is the FMIKit build I used: https://dev.azure.com/CATIA-Systems/FMIKit-Simulink/_build/results?buildId=673&view=artifacts&type=publishedArtifacts

EDIT: After manually adding the callback to the simulink block, I can confirm that deleting the block now cleans up the files. It does however not clean up any paths specified under build options.

Marvin-TMG avatar Oct 23 '20 09:10 Marvin-TMG

To make this function robust, I suggest the following implementation to automatically clean up any non-existing paths from the build info:

function removeBlockResources(block)
% Internal API - do not use

assert(strcmp(get_param(block, 'ReferenceBlock'), 'FMIKit_blocks/FMU'), 'Block is not an FMU')

userData = getUserData(block);

mdl = getfullname(bdroot(block));

unzipdir = FMIKit.getUnzipDirectory(block);

if ~strcmp('sfun_fmurun', userData.functionName)
    % remove S-function source
    sourceFile = which([userData.functionName '.c']);    
    delete(sourceFile);
    
    % remove S-function binary
    mexFunction = which([userData.functionName '.' mexext]);
    clear(mexFunction);
    delete(mexFunction);
end

% remove the unzip directory
[status, message, ~] = rmdir(unzipdir, 's');

% Update S-function sources
removeSrcParams(mdl, 'SimUserIncludeDirs');
removeSrcParams(mdl, 'SimUserSources');

% RTW sources
if strcmp(get_param(mdl, 'RTWUseSimCustomCode'), 'off')
    removeSrcParams(mdl, 'CustomInclude');
    removeSrcParams(mdl, 'CustomSource');
end

if status == 0
    warning(['Failed to remove unzip directory. ' message]);
end

end

function removeSrcParams(object, name)
param_value = get_param(object, name);

all_paths = strsplit(param_value, '" ');
for i = 1 : length(all_paths)
   all_paths{i} = strrep(strtrim(all_paths{i}), '"', '');
end

param_value = '';
for i = 1 : length(all_paths)
   if exist(all_paths{i}, 'dir') || exist(all_paths{i}, 'file')
       param_value = [param_value ' "' all_paths{i} '"' ];
   end
end
set_param(object, name, strtrim(param_value));
end

Marvin-TMG avatar Oct 23 '20 16:10 Marvin-TMG