Add desctructor to PwmOut

[LMESTM]

Issue #2017
Here's a proposal for review of ~PwmOut implementation

[LMESTM]

@0xc0170 can you add needed reviewers in cc ?

[0xc0170]

@c1728p9 @geky @bogdanm

Please review. We got in HAL xxx_free(), it is implemented for some platforms, for some just empty (thats another problem to be solved). There's one more PR for SerialBase ctor.

[0xc0170]

Why is dtor virtual ? I don't see any virtual method within this class, it's not intended to be a base class.

[geky]

Seconded.

Also the dtor can be inlined here, removing the need to add PwmOut.cpp. Since the function is just a single call to the c api there is no benefit to declaring the function externally.

Looking at this file, should pwmout_free be in a critical section?

[LMESTM]

Sure - I inspired from other classes and thought it could be a base class as well ... I'll simplify and make it a change in header file only ... when I'm back

[0xc0170]

Let us know once it's updated

[LMESTM]

back now: I followed recommendations to move all changes to the header file and put it under critical section. This is now rebased on top of master

[LMESTM]

@0xc0170 : update done

[LMESTM]

@0xc0170 - seems there are no conflicts for now

[0xc0170]

Thanks. We left similar comment to SerialBase dtor. This requires further design phase, how to handle destruction of objects that handles resources (peripherals). and what shall we allow/disallow within C++ drivers (copy constructor for instance). We will keep you updated.

[LMESTM]

Is it really better to keep it like this ?
For now as I see it, for basic usage, low layers resources like pin aren't freed, So if you create an object, delete, create the same new object again - it should fail because the pin is not free. At least such a basic use case can work with the proposed patch (For serial there are many derived class so I understand it's more complex).

[sg-]

Is it really better to keep it like this ?

For now, its better to leave it how it is since there are implication and documentation that also need to be considered.

[theotherjimmy]

Please reopen when unblocked.

[LMESTM]

well I'm not blocked, relevant issue is still open and valid afaik #3106