raspberrypi / pico-sdk

BSD 3-Clause "New" or "Revised" License
3.27k stars 844 forks source link

Function Naming for static void uart_set_irq_enables() - Inconsistent in SDK #1557

Open drankinatty opened 7 months ago

drankinatty commented 7 months ago

The naming for static void uart_set_irq_enables() is horribly inconsistent with the remaining functions in the SDK. All other similar functions are named ..._enabled(), but this one function has an 's' where the 'd' normally is.

For example see the Pico UART SDK Documentation Page and compare:

Or just about any other Hardware API function naming in the SDK, e.g.

This is a naming issue, so it's not the end of the world, but it is a significant Mental-Train Wreck where you are reading though code a come across this one "...enables()". Heaven knows the old 's', 'd' typo on a QWERTY keyboard isn't exactly unheard of...

The issue becomes how best to handle it? There is an argument to make that "it's been wonky this long, just leave it", but it is probably something that should be cleaned up. I'm not sure how best to do it other than rename it, set a #define to the old name and put out a deprecation notice that the old name may go away in a future release. You all know best.

Don't get me wrong, this isn't a complaint, the SDK is wonderful, just trying to do my part and pass along an opportunity to improve it. Keep up the great work.

peterharperuk commented 7 months ago

It's unlikely to be a typo. It enables both rx and tx fifo interrupts as requested.

kilograham commented 2 months ago

yeah; some things squeaked thru the naming consistency just before SDK1.0 ...

This should really be

uart_set_irqs_enabled and there should be adc_set_irq_enabled

generally we will just add a #define for the old spelling to the new correct one