PWM interface not consistent across implementations

[cibomahto]

The PWM interface appears to be only partially consistent across the various machine targets that implement it. I did a survey of the public interfaces for each of the implementations:

tinygo pwm spelunking.ods

Across implementations, there are some inconsistencies:

• The name of the PWM object type is not consistent across implementations. It is variously: timerType, PWM, pwmGroup, TCC, TIM
• The name of the exported PWM objects is not consistent. It is variously: Timerx, PWMx, TCCx, TIMx

And in specific implementations, there are some issues as well:

• machine_generic.go: Missing SetPeriod(), Period(), Counter(), and SetInverting(). Should probably be added since they seem to be part of the 'standard' interface
• machine_rp2.go: The PWM type is not exported, so references to the exported PWM objects cannot be stored in, for example, a project pin initialization list, without extra work
• machine_atsamd21.go, machine_atsamd51.go: Missing Period()
• machine_esp32xx_pwm.go: Missing SetPeriod(), Period(), and Counter()
• machine_nrf52xxx.go: Missing Period() and Counter()
• machine_stm32_tim.go: Missing Period(). Counter() is mis-named Count()

It's also not clear to me that there is a written standard for the PWM interface. I attempted to document it here: tinygo-org/tinygo-site#505

Assuming that it is desirable to regularize these, I would be happy to help out with any of the platforms that I have dev hardware for (I think everything except atsamdxx).

[deadprogram]

@cibomahto you are correct that things are non-consistent. This bit me recently, so I am quite in favor of some standardization.

Comments from other people please!

[aykevl]

Across implementations, there are some inconsistencies:

• The name of the PWM object type is not consistent across implementations. It is variously: timerType, PWM, pwmGroup, TCC, TIM

• The name of the exported PWM objects is not consistent. It is variously: Timerx, PWMx, TCCx, TIMx

This is more or less intentional, or rather: a result of how the hardware works.
Some hardware has more than one PWM type, and so the type name needs to be different between them. (The solution here would be to use interfaces in code that uses these types, instead of referring to the machine type directly).
The name of specific timers varies, which is unavoidable on a lot of hardware since timers can generally only connect to a particular set of output pins. So whichever way you do it, you need to have hardware specific code for this. And I think it's better to use the name the chip vendor uses for these, rather than try to standardize everything and make it harder to figure out which hardware timer is meant by that when looking up documentation for it.

And in specific implementations, there are some issues as well:

• machine_generic.go: Missing SetPeriod(), Period(), Counter(), and SetInverting(). Should probably be added since they seem to be part of the 'standard' interface

These may not be implemented on some hardware, perhaps because the hardware doesn't support it. If it's supported across all of them, feel free to make a PR to add to the interface.

• machine_rp2.go: The PWM type is not exported, so references to the exported PWM objects cannot be stored in, for example, a project pin initialization list, without extra work

Not sure why that is, probably a good idea to export it.

• machine_atsamd21.go, machine_atsamd51.go: Missing Period()
• machine_esp32xx_pwm.go: Missing SetPeriod(), Period(), and Counter()
• machine_nrf52xxx.go: Missing Period() and Counter()
• machine_stm32_tim.go: Missing Period(). Counter() is mis-named Count()

This is a matter of doing the implementation, feel free to send PRs :)

It's also not clear to me that there is a written standard for the PWM interface. I attempted to document it here: tinygo-org/tinygo-site#505

I made a WIP PR a long time ago, see: tinygo-org/tinygo-site#87 (feel free to reuse parts of it if you want)

[cibomahto]

Across implementations, there are some inconsistencies:

• The name of the PWM object type is not consistent across implementations. It is variously: timerType, PWM, pwmGroup, TCC, TIM
• The name of the exported PWM objects is not consistent. It is variously: Timerx, PWMx, TCCx, TIMx

This is more or less intentional, or rather: a result of how the hardware works. Some hardware has more than one PWM type, and so the type name needs to be different between them. (The solution here would be to use interfaces in code that uses these types, instead of referring to the machine type directly). The name of specific timers varies, which is unavoidable on a lot of hardware since timers can generally only connect to a particular set of output pins. So whichever way you do it, you need to have hardware specific code for this. And I think it's better to use the name the chip vendor uses for these, rather than try to standardize everything and make it harder to figure out which hardware timer is meant by that when looking up documentation for it.

Yes, I think a standardized PWM interface that each class implements is probably the best way to go here. Otherwise application code isn't portable between devices.

And in specific implementations, there are some issues as well:

• machine_generic.go: Missing SetPeriod(), Period(), Counter(), and SetInverting(). Should probably be added since they seem to be part of the 'standard' interface

These may not be implemented on some hardware, perhaps because the hardware doesn't support it. If it's supported across all of them, feel free to make a PR to add to the interface.

I would suggest including a reasonable subset of these in the interface, even if not every machine type implements them, to make the interface more useful for actual application programs. Of course every hardware is a special exception for this kind of thing.

I'm not a huge fan of either of these interfaces, but they are probably comparable:

• Micropython only exposes a subset of this: https://docs.micropython.org/en/latest/library/machine.PWM.html
• Arduino provides an even higher level and less functional abstraction: https://docs.arduino.cc/language-reference/en/functions/analog-io/analogWrite/

• machine_rp2.go: The PWM type is not exported, so references to the exported PWM objects cannot be stored in, for example, a project pin initialization list, without extra work

Not sure why that is, probably a good idea to export it.

• machine_atsamd21.go, machine_atsamd51.go: Missing Period()
• machine_esp32xx_pwm.go: Missing SetPeriod(), Period(), and Counter()
• machine_nrf52xxx.go: Missing Period() and Counter()
• machine_stm32_tim.go: Missing Period(). Counter() is mis-named Count()

This is a matter of doing the implementation, feel free to send PRs :)

It's also not clear to me that there is a written standard for the PWM interface. I attempted to document it here: tinygo-org/tinygo-site#505

I made a WIP PR a long time ago, see: tinygo-org/tinygo-site#87 (feel free to reuse parts of it if you want)

Good work!

[aykevl]

And in specific implementations, there are some issues as well:

• machine_generic.go: Missing SetPeriod(), Period(), Counter(), and SetInverting(). Should probably be added since they seem to be part of the 'standard' interface

These may not be implemented on some hardware, perhaps because the hardware doesn't support it. If it's supported across all of them, feel free to make a PR to add to the interface.

I would suggest including a reasonable subset of these in the interface, even if not every machine type implements them, to make the interface more useful for actual application programs. Of course every hardware is a special exception for this kind of thing.

Sorry I misread the above. machine_generic.go is mostly for the TinyGo Playground. It would certainly make sense to implement more of the PWM interface on here, if you'd like to contribute those (but they'll need accompanying changes on the playground side to be useful).