search

CANopenNode / CANopenEditor

CANopen Object Dictionary Editor
GNU General Public License v3.0
122 stars 60 forks source link

Crash if trying to change (increase) object data type that is mapped #129

Open trojanobelix opened 1 month ago

trojanobelix commented 1 month ago

Crash if trying to change SDO Data Type to DOMAIN in object directory Crash when saving the changes!

Reason: The object is RX mapped! Change type to DOMAIN will not fit in PDO 8 + 32 bit. Shoud happen with all changes were the mapping does not fit.

Possible reaction: Catch exception and abort saving

image

image

** Ausnahmetext ** System.ArgumentOutOfRangeException: Specified argument was out of the range of valid values. (Parameter 'ColumnSpan') at SourceGrid.Cells.Cell.set_ColumnSpan(Int32 value) at ODEditor.DevicePDOView2.UpdatePDOinfo(Boolean updatechoices) in C:\Git_lokal\CANopenEditor\EDSEditorGUI\DevicePDOView2.cs:line 466 at ODEditor.DeviceView.dispatch_updatePDOinfo() in C:\Git_lokal\CANopenEditor\EDSEditorGUI\DeviceView.cs:line 100 at ODEditor.MyTabUserControl.doUpdatePDOs() in C:\Git_lokal\CANopenEditor\EDSEditorGUI\MyTabUserControl.cs:line 44 at ODEditor.DeviceODView.PopulateObjectLists(EDSsharp eds_target) in C:\Git_lokal\CANopenEditor\EDSEditorGUI\DeviceODView.cs:line 191 at ODEditor.DeviceODView.ObjectSave() in C:\Git_lokal\CANopenEditor\EDSEditorGUI\DeviceODView.cs:line 585 at ODEditor.DeviceODView.Button_saveChanges_Click(Object sender, EventArgs e) in C:\Git_lokal\CANopenEditor\EDSEditorGUI\DeviceODView.cs:line 443 at System.Windows.Forms.Button.OnClick(EventArgs e) at System.Windows.Forms.Button.OnMouseUp(MouseEventArgs mevent) at System.Windows.Forms.Control.WmMouseUp(Message& m, MouseButtons button, Int32 clicks) at System.Windows.Forms.Control.WndProc(Message& m) at System.Windows.Forms.ButtonBase.WndProc(Message& m) at System.Windows.Forms.Control.ControlNativeWindow.WndProc(Message& m) at System.Windows.Forms.NativeWindow.Callback(IntPtr hWnd, WM msg, IntPtr wparam, IntPtr lparam)

CANopenNode commented 1 month ago

Problem is here: https://github.com/CANopenNode/CANopenEditor/blob/18610b31a2f6eb6b7d403cb41099aabf6b151e77/EDSEditorGUI/DevicePDOView2.cs#L466

ColumnSpan gets value 0 and it crashes. We should prevent crash with if or try statement or something.

The question is, how to handle this and similar situations:

First solution

Catch exception and abort saving

Second solution

UpdatePDOinfo() should verify the configuration and show warnings, similar as with some exporters. I prefer this approach because: it is simpler, more clear, more flexible for usage and it is user responsibility to fix the warnings.

We could use second solution (display warning) and also abort saving, but I think, that could be annoying for the user. Warning will remind him to fix PDO mapping and I think, this is enough.