[all] Unifying the way we report file related errors

[gaurab4163]

For consistency purpose, added a helper function to handle the file related errors for loaders and components.

To use it:

 FILE *f = fopen(filename.c_str(), "r");
 msg_error_when(!f)<<sofa::helper::message::UnableToOpenFile(filename.c_str());

Modified the usage in only "MeshTrian.cpp". With feedback, if I am in right direction, I will make changes to other loader and component accordingly.Solving issue #197

---

This PR:

• [ ] builds with SUCCESS for all platforms on the CI.
• [ ] does not generate new warnings.
• [ ] does not generate new unit test failures.
• [ ] does not generate new scene test failures.
• [ ] does not break API compatibility.
• [ ] is more than 1 week old (or has fast-merge label).

Reviewers will merge only if all these checks are true.

[sofabot]

Thank you for your pull request! Someone will soon check it and start the builds.

Note that by submitting a contribution to SOFA, you hereby accept and agree to the terms and conditions detailed in the associated DCO

[guparan]

Thanks for your PR @gaurab4163 :+1:

I'm not sure about the place (file and namespace) this function should be. @damienmarchal you have a clearer vision of logging and messages in SOFA, what do you think?

[damienmarchal]

Nice,

I'm happy someone make some proposal on this topic. I think we should factor the common messages, I don't think message.h/cpp is the right place for that.

I would put that either in separated place like containing only message:

• SofaCore/MessageLoader.cpp/h
• SofaCore/MessageBaseObject.cpp/h Or in the source code in the base class for the loaders.

Alternative: use a real text-framework that is suppose to externalise all the texts from the source code (to internationalize Sofa...but this wuold be time consuming work)

DM.

[gaurab4163]

@damienmarchal source code of base class for loader sounds right. Working on it :+1:

btw, i wondered why not use if and cout() than msg_error() and msg_error_when(). then came across #190 . It was very impressive. :+1:

[gaurab4163]

Moved the function to base class for loaders in last commit. Please review.

[guparan]

Why did you revert a commit in https://github.com/sofa-framework/sofa/pull/639/commits/ce882f37c3ade741d8bb47160dc42d0007cfc60f ?

We discussed this PR during our weekly dev meeting. The content is good, the placement is still perfectible. We agreed on creating a new class FileMessage in SofaKernel/framework/sofa/helper/messaging/FileMessage.{h,cpp} (new files) to contain your static function.

Is that ok for you? Could you implement it? Thank you for your reactivity :wink:

[gaurab4163]

Oh... when I made last commit, I meant to revert a37ba08. Had been awake for more than 40 hours(academics, tests, etc.) and evidently, wasn't thinking properly. (Slept like a baby right after last commit. :grin: ) Will adjust this in next commit.

Creating new files sounds right. I have another exam tomorrow. Will do right after it. :+1:

[guparan]

Hey @gaurab4163, do you have some time to continue this work?

[gaurab4163]

Hey @guparan , my final exams started last thursday and will continue until this friday(4th may). Just been crazy busy in last month overall. So, I ll get to it right after it.

[gaurab4163]

@guparan there is no messaging folder. did you mean SofaKernel/framework/sofa/helper/logging/FileMessage.{h,cpp} instead of SofaKernel/framework/sofa/helper/messaging/FileMessage.{h,cpp}

[epernod]

@gaurab4163 yes sorry for the missunderstood, the files should be placed in a new folder named: SofaKernel/framework/sofa/helper/messaging/ not in logging/ Could you change your commit and move the files. Thx.

[hugtalbot]

[ci-build][with-scene-tests]

[gaurab4163]

@epernod sure. just to make sure, the new class FileMessage will be placed in sofa::helper::messaging namespace too, right?

[epernod]

yes

[hugtalbot]

[ci-build][with-scene-tests]

[epernod]

thanks @gaurab4163 for the change. Could you just check the reviews, there is 2 files to remove: a .log and .view Then it is ok to go I think.

[damienmarchal]

[ci-build][with-scene-tests]

[damienmarchal]

This PR has been re-open in #669, So i close it, thank all for the work.