Add helper functions to HwPWM Instance

[jgartrel]

These functions allow for full control of the PWM configuration when direct register access is required and supplies a necessary function to reset the HwPWM to its default configuration.

A typical workflow might be:

1. takeOwnership of an available HwPWM instance through the standard API
2. lookup the baseAddr of that HwPWM instance if direct configuration using NRF_PWMx is required
3. configure the HwPWM
4. addPin(s) as necessary to the HwPWM
5. enable the HwPWM
6. stop the HwPWM when finished
7. removeAllPins from the HwPWM
8. releaseOwnership and return the HwPWM instance back to the available pool

[hathach]

removeAllPins is OK however baseAddr() is not accepted, since that could cause incosistent between private value of HwPWM object when configured externally.

[jgartrel]

@hathach, It seems to me it would be nice to allow a programmer to takeOwnership of a HwPWM to use it for some purpose , then return it back to the available pool when done. Without this, how would one take ownership of a HwPWM and use it manually if HwPWM does not expose the actual NRF_PWM baseAddr?

To do this today, my code needs to make an assumption that the following mapping exists:

HardwarePWM HwPWM0(NRF_PWM0);
HardwarePWM HwPWM1(NRF_PWM1);
HardwarePWM HwPWM2(NRF_PWM2);
HardwarePWM HwPWM3(NRF_PWM3);

and then maintain a separate hardcoded private mapping such that, when I take ownership of HwPWMx, I use the corresponding NRF_PWMx. However, this is a BIG assumption and could easily go wrong if this mapping were to change between boards or a future revision of the Aradfruit_nRF52_Arduino core.

It would be much more robust if, after takeOwnership of the HwPWMx I could just ask it what NRF_PWM to use for direct configuration.

Yes it is true that you could shoot yourself in the foot, but to be honest, you can already do that if you takeOwnership of a HwPWM, then 'addPin', and then releaseOwnership. Then next process to 'takeOwnership' will then be unaware that a pin mapping exists and have inconsistent behavior. In fact, prior to this merge request and the removeAllPins() function, you would have to add all the pins and then remove all of the pins in order to ensure your HwPWM was properly reset back to a know good state. Additionally, you could just use the NRF_PWMx interface directly and poof, inconsistent behavior.

IMO, preventing programmers from accessing the value of _pwm via baseAddr is an artificial constraint that does not provide any guarantees of consistency in the HwPWM interface. Whether you use the baseAddr() method for nefarious purposes or use NRF_PWMx directly there is nothing that the Adafruit_nRF52_Arduino core could do to prevent inconsistencies that would arise.

If we allow this addition, we open the door for a whole host of new creative uses of the HwPWM, much more inline with Nordic's intentions for the flexibility of the PWM peripheral.

Look at Tone.cpp in this repo, this is a great example:

• Adafruit_nRF52_Arduino/cores/nRF5/Tone.cpp

  Lines 47 to 48 in ec8b49a

  	static NRF_PWM_Type * const _PWMInstance = NRF_PWM2;
  	static HardwarePWM * const _HwPWM = HwPWMx[2];

Instead of dynamically selecting a HwPWM from the pool, the programmer must hard code a preferred HwPWM2 so it could be access directly with a static mapping on Line 47 to NRF_PWM2. This code would be so much cleaner and less tightly coupled if HwPWM exposed baseAddr().

As time goes on, we should work to ensure that HwPWM does properly reset its NRF_PWM and pin configuration to a know good state, so we CAN offer a guarantee of consistency to the next consumer to takeOwnership of a HwPWM.

Does that make sense or would you still suggest that I remove baseAddr()?

[jgartrel]

@hathach Does the above make sense or would you still suggest that I remove baseAddr()?

[hathach]

baseAddr() still needs to be removed. All changes need be done via class member function. You don't need to use this Hardware PWM though, just use the bare NRF_PWM if you prefere to manage your own PWM instance.

[jgartrel]

@hathach Thank you for considering the addition. I will remove the baseAddr() function. FWIW, I DO still have to use this HardwarePWM, so that it is reserved and removed from available use. That also means I have to maintain the dangerous parallel mapping described above or a perpetual fork of this repository. If you have a better idea on how to ensure that future releases of the BSP do not break that parallel mapping, please let me know.

On a side note: Tone.cpp (in this BSP) currently violates the rule about causing "inconsistent between private value of HwPWM object when configured externally". It directly manipulates NRF_PWM2 in a way that cannot be detected (or reset) by the HwPWM interface (e.g. setting NRF_PWM2->SHORTS). Once the Tone class is used (and it releases ownership of HwPWM[2]), HwPWM[2] cannot be used again for anything else without risk. If I have some free time, I will try and submit a pull request to resolve as many of those that I can find.

[hathach]

Tone.cpp is written by others and I am too lazy to wrap it up with HwPWM. I may have a second thought on getBase() however, I currently don't have time to revise this at the moment. Maybe later.

[hathach]

look good, thank you for your PR