View Issue Details

Related ChangesetsJump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0032125Open CASCADEOCCT:Application Frameworkpublic2021-02-10 10:342023-02-03 05:23
ReporterabvAssigned Tovro  
PrioritynormalSeverityminor 
Status closedResolutionfixed 
Target Version7.6.0Fixed in Version7.6.0 
Summary0032125: Application Framework, OCAF - need to have adequate message if document cannot be saved due to unrecognized format
DescriptionIn DRAW:

> source samples/tcl/xde.tcl
> SaveAs D ./xde.cbf
Error saving document: Write failure

The failure is apparently because the format of the document set at creation in the script as "XCAF" does not match any of the supported persistence formats (e.g. "BinXCAF").

The error message should provide indication on the reason of the failure (unrecognized format).
Steps To ReproduceSee Description section.
TagsNo tags attached.
Test case numberNot required

Relationships

related to 0032531 closedsmoskvin Community Application Framework - There should be methods to get explanations of PCDM_StoreStatus and PCDM_ReaderStatus values 

Activities

git

2021-07-19 19:23

administrator   ~0102660

Branch CR32125 has been created by vro.

SHA-1: c723a13eee36471898b6e22478cead511e9a3ecf


Detailed log of new commits:

Author: vro
Date: Mon Jul 19 19:22:36 2021 +0300

    0032125: Application Framework, OCAF - need to have adequate message if document cannot be saved due to unrecognized format
    
    A list of storage statuses (PCDM_StoreStatus) is extended for PCDM_SS_UnrecognizedFormat. It is set if
    - extension of a document file name is not defined (the extension doesn't correspond to any declared file formats) or
    - a storage driver is not found for the document storage format.
    
    Modifications:
    CDF_StoreList.cxx: a unique exception is raised in case of the driver is not found by the document format.
    PCDM_StoreStatus.hxx: a new enumeration value is added - PCDM_SS_UnrecognizedFormat.
    DDocStd_ApplicationCommands.cxx: processing of a new enumeration value by the draw-command.

git

2021-07-19 21:12

administrator   ~0102662

Branch CR32125 has been updated by vro.

SHA-1: 01b392363e6e525a62af078def15e7521d5b3d5b


Detailed log of new commits:

Author: vro
Date: Mon Jul 19 21:11:50 2021 +0300

    0032125: Application Framework, OCAF - need to have adequate message if document cannot be saved due to unrecognized format
    
    Modifications:
    - XDEDRAW.cxx: processed a new enumeration value PCDM_SS_UnrecognizedFormat by saveDoc draw-command.

git

2021-07-20 05:03

administrator   ~0102667

Branch CR32125_1 has been created by vro.

SHA-1: fbeed435b4536e4c6ec287cc267b40328901a954


Detailed log of new commits:

Author: vro
Date: Tue Jul 20 05:02:43 2021 +0300

    0032125: Application Framework, OCAF - need to have adequate message if document cannot be saved due to unrecognized format
    
    A list of storage statuses (PCDM_StoreStatus) is extended for PCDM_SS_UnrecognizedFormat. It is set if
    - extension of a document file name is not defined (the extension doesn't correspond to any declared file formats) or
    - a storage driver is not found for the document storage format.
    
    Modifications:
    - CDF_StoreList.cxx: a unique exception is raised in case of the driver is not found by the document format.
    - PCDM_StoreStatus.hxx: a new enumeration value is added - PCDM_SS_UnrecognizedFormat.
    - DDocStd_ApplicationCommands.cxx: processing of a new enumeration value PCDM_SS_UnrecognizedFormat by SaveAs draw-command.
    - XDEDRAW.cxx: processing of a new enumeration value PCDM_SS_UnrecognizedFormat by saveDoc draw-command.

kgv

2021-07-20 12:05

developer   ~0102674

Last edited: 2021-07-20 12:06

 
+    catch (Standard_ProgramError const& anException) {
+      CAUGHT(anException, aStatusAssociatedText, TCollection_ExtendedString("driver not found; reason:"));
+      status = PCDM_SS_UnrecognizedFormat;

Using general-purpose exception to be treated as specific error looks confusing (cannot say if it is error-prone in this context).

void CDF_Store::Realize (const Message_ProgressRange& theRange)
{
  Standard_ProgramError_Raise_if(!myList->IsConsistent(),"information are missing");
  Handle(CDM_MetaData) m;
  myText = "";
  myStatus = myList->Store(m, myText, theRange);
  if(myStatus==PCDM_SS_OK) myPath = m->Path();
}
...
PCDM_StoreStatus TDocStd_Application::SaveAs (const Handle(TDocStd_Document)& theDoc,
                                              const TCollection_ExtendedString& path,
                                              const Message_ProgressRange& theRange)
{
...
  try {
    OCC_CATCH_SIGNALS
    storer.Realize (theRange);
  }
  catch (Standard_Failure const& anException) {
    if (!MessageDriver().IsNull()) {
      TCollection_ExtendedString aString (anException.GetMessageString());
      MessageDriver()->Send(aString.ToExtString(), Message_Fail);
    }
  }

CDF_Store stores detailed error description as class field CDF_Store::myText, which is inaccessible to the used of TDocStd_Application::SaveAs() as only PCDM_StoreStatus enumeration value is returned.

Maybe it worth redirecting this message on failure state to the MessageDriver like it is done in case of unhandled exception?
Or at least with Message_Trace level...

kgv

2021-07-20 12:07

developer   ~0102675 

--- a/src/PCDM/PCDM_StoreStatus.hxx
+++ b/src/PCDM/PCDM_StoreStatus.hxx
@@ -27,7 +27,8 @@ PCDM_SS_Failure,
 PCDM_SS_Doc_IsNull,
 PCDM_SS_No_Obj,
 PCDM_SS_Info_Section_Error,
-PCDM_SS_UserBreak
+PCDM_SS_UserBreak,
+PCDM_SS_UnrecognizedFormat

Would be nice adding documentation to each enumeration value (new and old ones) in scope of this patch.

git

2021-09-01 11:47

administrator   ~0103613

Branch CR32125_2 has been created by vro.

SHA-1: 757d8366caf54c0ea13387433dfd96f28586b790


Detailed log of new commits:

Author: vro
Date: Wed Sep 1 11:47:41 2021 +0300

    0032125: Application Framework, OCAF - need to have adequate message if document cannot be saved due to unrecognized format
    
    A list of storage statuses (PCDM_StoreStatus) is extended for PCDM_SS_UnrecognizedFormat. It is set if
    - extension of a document file name is not defined (the extension doesn't correspond to any declared file formats) or
    - a storage driver is not found for the document storage format.
    
    Modified:
    CDF_StoreList.cxx: the mechanism of raising and catching exceptions is replaced by direct setting the status and error message for each particular error.
    PCDM_StoreStatus.hxx: a new enumeration value is added - PCDM_SS_UnrecognizedFormat. Also, a short explanation is added to each enumeration value.
    DDocStd_ApplicationCommands.cxx: processing of a new enumeration value by the draw-command.
    XDEDRAW.cxx: processed a new enumeration value PCDM_SS_UnrecognizedFormat by saveDoc draw-command.
    
    Deleted:
    CDF_MetaDataDriverError.hxx: It is not used anywhere and it seems it will not be used by someone.

git

2021-09-01 12:28

administrator   ~0103615

Branch CR32125_2 has been updated by vro.

SHA-1: 1478c0fb2c861369357b6c95db0b4c605232e2f4


Detailed log of new commits:

Author: vro
Date: Wed Sep 1 12:27:56 2021 +0300

    // Added a forgotten file XDEDRAW.cxx

vro

2021-09-02 14:11

developer   ~0103675

Indeed, throwing exceptions in order to set a message looks too sophisticated. Corrected. Also, I removed a not used CDF_MetaDataDriverError exception class, which is not used. Documentation on PCDM_StoreStatus is added too.

http://jenkins-test-occt/view/CR32125-CR32125-VRO/view/COMPARE/. There is a failing test-case "opengl drivers d3dhost", which is obviously out of scope of this patch.

kgv

2021-09-02 14:26

developer   ~0103676

Patch looks fine from my point of view (untested).

mpv

2021-09-03 08:59

developer   ~0103700

Reviewed.

OCCT branch: CR32125_2
Products branch: NOTHING

vro

2021-09-03 09:01

developer   ~0103701

Products branch: CR32125_2

// There is a very little change in WIG: I deleted CDF_MetaDataDriverError class.

mpv

2021-09-03 09:05

developer   ~0103702

Ok.

OCCT branch: CR32125_2
Products branch: CR32125_2

smoskvin

2021-09-04 14:25

administrator   ~0103757

Combination -
OCCT branch : IR-2021-09-03
master SHA - f26ee38f2a309ffbf7de4eebbcef2c5a5c57d84e
a87b7ddc8cb44606b91e3f37113847c3f5f50fdc
Products branch : IR-2021-09-03 SHA - 87cca1a8f3dd94387a936b9d49f5bd719c69cf4d
was compiled on Linux, MacOS and Windows platforms and tested in optimize mode.

Number of compiler warnings:
No new/fixed warnings

Regressions/Differences/Improvements:
No regressions/differences

CPU differences:
Debian80-64:
OCCT
Total CPU difference: 17495.94000000042 / 17441.55000000031 [+0.31%]
Products
Total CPU difference: 11565.440000000113 / 11534.270000000102 [+0.27%]
Windows-64-VC14:
OCCT
Total CPU difference: 19307.140625 / 19200.421875 [+0.56%]
Products
Total CPU difference: 12917.859375 / 12874.53125 [+0.34%]


Image differences :
No differences that require special attention

Memory differences :
No differences that require special attention

git

2021-09-04 14:59

administrator   ~0103787

Branch CR32125 has been deleted by mnt.

SHA-1: 01b392363e6e525a62af078def15e7521d5b3d5b

git

2021-09-04 14:59

administrator   ~0103788

Branch CR32125_1 has been deleted by mnt.

SHA-1: fbeed435b4536e4c6ec287cc267b40328901a954

git

2021-09-04 14:59

administrator   ~0103789

Branch CR32125_2 has been deleted by mnt.

SHA-1: 1478c0fb2c861369357b6c95db0b4c605232e2f4

BenjaminBihler

2021-09-20 09:35

updater   ~0104220

Thank you for your work, but my intention has been even more broad. With "user" I did not mean the software developer using OCCT, but the one using the final software. When reading or writing fails, it would be nice to display an error message dialog to him showing the message "Driver is not found for the defined file format."

For that I would need two methods

TCollection_ExtendedString getMessage(PCDM_ReaderStatus status)

and

TCollection_ExtendedString getMessage(PCDM_StoreStatus status).

Then I could paste the return value of these messages into the dialog. Do you agree? Would that be a valuable addition?

mpv

2021-09-20 09:59

developer   ~0104221

Dear Benjamin,

Basing on our experience, the lower level parts of the application should not generate end-user messages. Especially this concerns quite universal OCCT libraries, because such messages are very application and user specific.

The message "Driver is not found for the defined file format." could not help the user who even don't know what file format it is related to.

So, we propose an error as the enumeration value, understandable for developer of the user interface and developer should correspond a suitable message (perhaps, using native language), understandable for the user in particular case.

BenjaminBihler

2021-09-20 10:10

updater   ~0104222

I agree, thank you.

BenjaminBihler

2021-09-20 14:15

updater   ~0104231

To be honest, I am not sure whether I agree. There are SaveAs(...) methods of TDocStd_Application that take a parameter like this: TCollection_ExtendedString& theStatusMessage.

What is this status message for? For the programmer? Wouldn't an enum value serve the purpose better than a TCollection_ExtendedString? But if you want to use strings, then why not everywhere?

mpv

2021-09-20 18:55

developer   ~0104243

There are methods "SaveAs" with nearly the same functionality inside, but with theStatusMessage argument inside and without it.
I agree, here we have some inconsistency, but this is because of "historical" reasons for many years. I would remove methods with theStatusMessage, normally they are not needed, but this will change API and could cause changes in old programs.
So, this is an exception, enums are preferable anyway.

BenjaminBihler

2021-09-21 08:35

updater   ~0104250

Okay.

Related Changesets

occt: master 632deee0

2021-09-01 08:47:41

vro


Committer: bugmaster Details Diff		 0032125: Application Framework, OCAF - need to have adequate message if document cannot be saved due to unrecognized format

A list of storage statuses (PCDM_StoreStatus) is extended for PCDM_SS_UnrecognizedFormat. It is set if
- extension of a document file name is not defined (the extension doesn't correspond to any declared file formats) or
- a storage driver is not found for the document storage format.

Modified:
CDF_StoreList.cxx: the mechanism of raising and catching exceptions is replaced by direct setting the status and error message for each particular error.
PCDM_StoreStatus.hxx: a new enumeration value is added - PCDM_SS_UnrecognizedFormat. Also, a short explanation is added to each enumeration value.
DDocStd_ApplicationCommands.cxx: processing of a new enumeration value by the draw-command.
XDEDRAW.cxx: processed a new enumeration value PCDM_SS_UnrecognizedFormat by saveDoc draw-command.

Deleted:
CDF_MetaDataDriverError.hxx: It is not used anywhere and it seems it will not be used by someone.		 Affected Issues
0032125
rm - src/CDF/CDF_MetaDataDriverError.hxx Diff File
mod - src/CDF/CDF_StoreList.cxx Diff File
mod - src/CDF/FILES Diff File
mod - src/PCDM/PCDM_StoreStatus.hxx Diff File
mod - src/TDocStd/TDocStd_Application.cxx Diff File
mod - src/XDEDRAW/XDEDRAW.cxx Diff File

occt-products: master 87cca1a8

2021-09-01 13:13:14

vro


Committer: bugmaster Details Diff		 0032125: Application Framework, OCAF - need to have adequate message if document cannot be saved due to unrecognized format Affected Issues
0032125
mod - diff Diff File

Issue History

Date Modified Username Field Change
2021-02-10 10:34 abv New Issue
2021-02-10 10:34 abv Assigned To => mpv
2021-02-10 10:42 kgv Summary OCAF - need to have adequate message if document cannot be saved due to unrecognized format => Application Framework, OCAF - need to have adequate message if document cannot be saved due to unrecognized format
2021-07-19 19:23 git Note Added: 0102660
2021-07-19 21:12 git Note Added: 0102662
2021-07-20 05:03 git Note Added: 0102667
2021-07-20 07:08 vro Status new => resolved
2021-07-20 07:08 vro Steps to Reproduce Updated
2021-07-20 12:05 kgv Note Added: 0102674
2021-07-20 12:06 kgv Note Edited: 0102674
2021-07-20 12:07 kgv Note Added: 0102675
2021-08-11 12:26 mpv Assigned To mpv => vro
2021-08-11 12:26 mpv Status resolved => assigned
2021-08-12 12:44 kgv Relationship added related to 0032531
2021-09-01 11:47 git Note Added: 0103613
2021-09-01 12:28 git Note Added: 0103615
2021-09-02 14:11 vro Note Added: 0103675
2021-09-02 14:11 vro Assigned To vro => kgv
2021-09-02 14:11 vro Status assigned => resolved
2021-09-02 14:11 vro Steps to Reproduce Updated
2021-09-02 14:26 kgv Note Added: 0103676
2021-09-02 14:26 kgv Assigned To kgv => mpv
2021-09-03 08:59 mpv Note Added: 0103700
2021-09-03 08:59 mpv Assigned To mpv => bugmaster
2021-09-03 08:59 mpv Status resolved => reviewed
2021-09-03 09:01 vro Note Added: 0103701
2021-09-03 09:05 mpv Note Added: 0103702
2021-09-04 14:25 smoskvin Note Added: 0103757
2021-09-04 14:25 smoskvin Status reviewed => tested
2021-09-04 14:36 bugmaster Changeset attached => occt master 632deee0
2021-09-04 14:36 bugmaster Status tested => verified
2021-09-04 14:36 bugmaster Resolution open => fixed
2021-09-04 14:59 git Note Added: 0103787
2021-09-04 14:59 git Note Added: 0103788
2021-09-04 14:59 git Note Added: 0103789
2021-09-04 16:26 smoskvin Test case number => Not required
2021-09-20 09:35 BenjaminBihler Note Added: 0104220
2021-09-20 09:59 mpv Note Added: 0104221
2021-09-20 10:10 BenjaminBihler Note Added: 0104222
2021-09-20 14:15 BenjaminBihler Note Added: 0104231
2021-09-20 18:55 mpv Note Added: 0104243
2021-09-21 08:35 BenjaminBihler Note Added: 0104250
2021-12-17 18:41 bugmaster Changeset attached => occt-products master 87cca1a8
2021-12-17 18:41 vro Assigned To bugmaster => vro
2021-12-17 18:41 vro Status closed => verified
2023-02-03 05:23 vglukhik Status verified => closed