Hacker News new | past | comments | ask | show | jobs | submit login

You should contribute these improvements upstream then.

The Arudino developers are not trying to write slow code on purpose. Mocking them on hackernews doesn't solve anything. I'm sure they would greatly appreciate you pointing out how slow this code is and offering your improved implementation.




> You should contribute these improvements upstream then.

You honestly think they'd accept changes that would break literally every single library using these functions? Somehow I find that hard to believe.

> The Arudino developers are not trying to write slow code on purpose.

Of course they didn't. No reasonable person goes out of their way to make a terrible API. That's why I'm confused and irritated about how they managed to do such a bad job.

All the inputs are bare integers, pins and mode/state, so you get no warning whatsoever when you get it wrong. I've seen posts from people confused because they tried to digitalWrite pins A6 or A7 on a Nano which is accepted by the compiler and runs, but doesn't work correctly because those pins aren't on a digital port. The other analog pins are on a digital port, so work fine.

It gets better. Let's say I digitalWrite with a pin number higher than 19 on a Nano, what happens? Well, it will start off by doing lookups in its pin tables [0]. How does it do the lookup? It just adds the user's value to the lookup tables' addresses [1], and reads that address. On the Nano, these tables are 20 items long [3]. It's going to read god knows what from your flash, and use whatever gibberish it received to decide what to do!

And, because of the need to use inline assembly for the lookups, the compiler can't reason about the data in the tables, and so can't optimize out the lookups, nor can it properly optimize code that relies on those lookups. So the result is it ends up needlessly consuming your limited flash space.

And this is an API that's supposed to be used by beginners! Beginners are the ones most likely to get it wrong because they don't know better.

They're using a C++ compiler in C++11 mode, they're not stuck in the dark ages! Why use a bare integer to represent the pin, a pin's IO mode, or the pin state, or the analog reference voltage source (which, by the way, also makes no attempt to ensure it's a valid value [4])? At the very least they could have used enum classes instead of bare integers so users get a compiler error when they fuck up.

[0] https://github.com/arduino/ArduinoCore-avr/blob/master/cores...

[1] https://github.com/arduino/ArduinoCore-avr/blob/master/cores... [2]

[2] Because of the separate memory spaces, it has to use inline assembly to do this. The pgm_read_byte macro from the AVR header is what does that here.

[3] https://github.com/arduino/ArduinoCore-avr/blob/master/varia...

[4] https://github.com/arduino/ArduinoCore-avr/blob/master/cores...




Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact

Search: