technoblogy/attiny10core

Suggestions to improve accessibility

Closed this issue · 5 comments

SpenceKonde commented
  1. Example code - you have some examples on your blog. Create a "dummy" library (ie, named ATtiny10Core - or with whatever capitalization you like; ATtiny seems to be the most "correct", I only have ATTinyCore capitalized like that for historical reasons), and you can put the sketch dolders in the examples folder in there; be sure to remind people that this exists in the readme. Surely it would give some blog cred to be able to say that these examples are included with the core, right? I think the most important example would be an implementation of everyone's favorite sketch, blink, cause that's what everyone wants to be able to do after installing a core to prove that the core works, the programmer works, and they have basic functionality intact: compile and upload blink, and see the LED blinking.
    I provide examples for my cores this way, and based on issues people have opened, they actually look at the examples too!
  2. Get it to automatically include <avr/io.h> and <stdint.h>,
  3. Mention the board manager installation option in the readme.
  4. If it does not adversely impact the size of the generated sketch, I'd add the setup() and loop() stuff to main.cpp
  5. Building on above, I'd also put in a menu to choose 8 MHz or 1 MHz and define F_CPU and if 8 MHz, switch off the prescale before calling setup
  6. Consider digitalWrite/digitalRead/pinMode, implemented as the ____Fast() ones are,
void badArg(const char*) __attribute__((error("")));
// badArg is when we can determine at compile time that an argument is inappropriate.
#define HIGH 1
#define LOW 0
inline __attribute__((always_inline)) void digitalWrite(uint8_t pin, uint8_t value)
{
  if(!__builtin_constant_p(pin))
    badArg("digital pin must be constant known at compile time");
  if(pin > 3)
    badArg("digital pin must exist");
  if (value == LOW) {
    PORTB &= ~(1<<pin);
  } else {
    PORTB |= 1<<pin;
  }
}

Should compile down to a single sbi or cbi. for pinMode, you'd get 1 instruction for output, 2 for input/input_pullup

#define INPUT 0
#define OUTPUT 1
#define INPUT_PULLUP 2
inline __attribute__((always_inline)) void pinMode(uint8_t pin, uint8_t mode)
{
  if(!__builtin_constant_p(pin))
    badArg("digital pin must be constant known at compile time");
  if(pin > 3)
    badArg("digital pin must exist");
  if(!__builtin_constant_p(mode))
    badArg("pin mode must be constant known at compile time");
  if (mode == OUTPUT) {
    DDRB |= 1<<pin;
  } else {
    // set PUE first so no possibility of a glitch when going from outputting high to input pullup
    if {mode == INPUT_PULLUP) {
     PUEB |= 1<<pin;
    } else {
      PUEB &= ~(1<<pin);
    }
    DDRB &= ~(1<<pin);
  }
}

or - this which should always compile down to 1 instruction each, and protects someone from changing an INPUT_PULLUP straight to OUTPUT and writing it LOW, then going to sleep to save power with the pullup on and port outputting LOW (ie, "draw an extra Vcc/Rpu current" mode)

#define INPUT 0
#define OUTPUT 1
#define INPUT_PULLUP 2
#define PULLUP 1
#define NOPULLUP 0

inline __attribute__((always_inline)) void pinMode(uint8_t pin, uint8_t mode)
{
  if(!__builtin_constant_p(pin))
    badArg("digital pin must be constant known at compile time");
  if(pin > 3)
    badArg("digital pin must exist");
  if(!__builtin_constant_p(mode))
    badArg("pin mode must be constant known at compile time");
  if(mode > 1)
    badArg("use pinPullup() to control internal pullups");
  if (mode == INPUT) {
    DDRB &= ~(1<<pin);
  } else {
    DDRB |= 1<<pin;
  }
}

inline __attribute__((always_inline)) void pinPullup(uint8_t pin, uint8_t mode)
{
  if(!__builtin_constant_p(pin))
    badArg("digital pin must be constant known at compile time");
  if(pin > 3)
    badArg("digital pin must exist");
  if(!__builtin_constant_p(mode))
    badArg("pin mode must be constant known at compile time");
  if(mode > 1)
    badArg("second argument must be PULLUP or NOPULLUP");
  if (mode == NOPULLUP) {
    PUEB &= ~(1<<pin);
  } else {
    PUEB |= 1<<pin;
  }
}

for digitalRead() this generally gets turned into a single sbic/sbis instruction:


inline __attribute__((always_inline)) uint8_t digitalRead(uint8_t pin)
{
  if(!__builtin_constant_p(pin))
    badArg("digital pin must be constant known at compile time");
  if(pin > 3)
    badArg("digital pin must exist");
  return !!(PINB & 1<<pin)
}

One can of course argue about whether other things should be pulled in or not (I tend to by more on the side of "don't pull in garbage, it's a 1k chip, you can't program it like an Arduino" - I only advocate these on the grounds that they compile down to a single 1-clock atomic instruction and, frankly, hand-written direct port interaction) has a non-zero chance of doing it worse, especially when written by people who aren't very familiar with the architecture. Someone who doesn't understand sbi/cbi but does have basic programming experience ("a little knowledge is a dangerous thing" effect( could easily screw it up by assuming that PORTB |= 1<<2 was worse than PORTB = 1<<2 when it is known that PORTB currently is set to 0x04 or 0x00 "well it's a read-modify-write, better to just set it" and waste an extra word on the ldi - assuming there's space in the registers for that; more if there isn't) and because it does make the code more readable.

As an aside - the avrlibc busywait delay in util/delay.h _delay_ms() is surprisingly flash-efficient on these! I've got no hardware to test it on, but I was impressed that blink using that was only like 80-some bytes. Though I haven't the faintest idea what it thinks the CPU frequency is! (sure, I could figure out what it thinks it is from the generated code, but have other things I need to do today)

technoblogy commented

Great suggestions - thanks!

technoblogy commented

Example code - you have some examples on your blog. Create a "dummy" library and you can put the sketch folders in the examples folder in there;

Good idea, but I can't quite figure out where they should go. I can't find them in your cores on GitHub.

SpenceKonde commented
technoblogy commented

Thanks - got it.

technoblogy commented

Thanks for the suggestions!

Example code

Added a couple of examples in 2.0.0. Will try to add more later.

Get it to automatically include <avr/io.h> and <stdint.h>

Done in 2.0.0.

Mention the board manager installation option in the readme.

Done.

If it does not adversely impact the size of the generated sketch, I'd add the setup() and loop() stuff to main.cpp

Done.

Building on above, I'd also put in a menu to choose 8 MHz or 1 MHz and define F_CPU and if 8 MHz, switch off the prescale before calling setup

Good suggestion. Done.

Consider digitalWrite/digitalRead/pinMode, implemented as the ____Fast() ones are

Haven't done this for 2.0.0, and think it's possibly easier to say that no Arduino functions are provided, rather than providing a few that can be written compactly. But will think about it more for the future.