Porting Arduboy2 library to SAMD

For adding new hardware to the Arduino IDE, you should use the following documents:

Edit: Also this:
https://playground.arduino.cc/Main/BoardsManagerSupport/

2 Likes

@MLXXXp thank you. I am aware of the Arduino way of adding board support but I was rather after any Arduboy community preferences on e.g. whether it should be added to an existing repo or given a standalone, both short term (as WIP) and longer term.


I’m pretty sure I tried without brackets first. I’ll go back and try again.

@MLXXXp manages Arduboy2 github so I’d imagine if you made a push request there he would review it.

I dunno my general feeling it’s enough of a jump that it probably warrants it’s own repo.

Maybe call it Arduboy3? :smiley:

1 Like

I think it’s just hardware specification files, not game compiling libraries, so probably needs a separate repo if there isn’t already a boards.txt for Arduboy (wouldn’t it be easy to do one?)

I stated earlier in this topic:

However, if you wish, I don’t mind having this work on a branch in my Arduboy2 repository, without it interfering with the master branch. I could make you a repository collaborator so you could maintain that branch.


No. As I’ve already stated in this topic:

If it’s renamed to something else, such as Arduboy3, you then have to modify each sketch in order for it to compile against the new hardware. It should be part of Arduboy2 using conditional compiles.

However, if the new hardware ends up being different enough that many Arduboy sketches wont compile or work properly without modification, such as having a new display that’s not 100% backwards compatible, then that’s a different story.

There is. The URL to add it to the Arduino IDE is:
https://arduboy.github.io/board-support/package_arduboy_index.json

1 Like

I would like the branch of Arduboy2 that is SAMD compatible to be merged back in to the master if possible, as I think we have discussed before.

My current line of enquiry is about the 3rd party hardware specification and the files that go with it. I would like to add them to an existing repo (thanks for the json link @MLXXXp) but will set up a separate one or fork the original to add the SAMD board that I have developed and should also be compatible with other SAMD21 based boards.

Yes, do that for now until if/when hardware becomes official or popular. You don’t currently plan to actually have it listed by Arduino, do you?

Nope_______

1 Like

I just commented out the #include avr/eeprom, eeprom_write_byte etc in the Utils/Eeprom.h and then hit this:

In file included from     C:\Users\...\AppData\Local\Temp\arduino_build_910817\sketch\Utils/Utils.h:8:0,
                 from C:\Users\...\AppData\Local\Temp\arduino_build_910817\sketch\Utils.h:1,
                 from C:\Users\...\AppData\Local\Temp\arduino_build_910817\sketch\SaveSystem.h:22,
                 from C:\Users\...\AppData\Local\Temp\arduino_build_910817\sketch\GameContext.h:24,
                 from C:\Users\...\AppData\Local\Temp\arduino_build_910817\sketch\TileGridRenderer.cpp:19:
C:\Users\...\AppData\Local\Temp\arduino_build_910817\sketch\Utils/Random.h: In function 'long unsigned int generateRandomSeed()':
C:\Users\...\AppData\Local\Temp\arduino_build_910817\sketch\Utils/Random.h:23:19: error: 'power_adc_enable' was not declared in this scope
  power_adc_enable(); // ADC on
                   ^
C:\Users\...\AppData\Local\Temp\arduino_build_910817\sketch\Utils/Random.h:26:2: error: 'ADCSRA' was not declared in this scope
  ADCSRA |= _BV(ADSC); // start conversion (ADMUX has been pre-set in boot())
^

So I’m wondering if there’s likely to be a game to test compilation on that has fewer calls to specific registers in the ATMEGA32U and would be more reliant on the Arduboy2 library to do these things for it. Any tips on which game might be well suited for this early library compilation testing?


EDIT: I commented my way around that issue but… also, it seems that this sketch is trying to use random() without parameters and Arduino doesn’t seem to like it:

C:\Users\...\AppData\Local\Temp\arduino_build_910817\sketch\GameplayState.cpp: In static member function 'static void GameplayState::pickMineLocations(GameplayState::TileGrid&, uint8_t)':
GameplayState.cpp:266:50: error: no matching function for call to 'random()'
    const uint8_t x = static_cast<uint8_t>(random()) % Width;

This is just for the function that generates a random seed for the PRNG.
Any source of noise (e.g. an unconnected pin) would be a suitable substitute.

I wrote that before Aruboy2Base::generateRandomSeed was introduced to Arduboy2.
In fact the two functions should be (more or less) identical.

More likely that random() doesn’t actually exist in your environment because it’s from avr-libc, not Arduino.
(Arduino introduces two overloads, but not the original parameterless version.)

This is one of the functions I implement in my avr-libc port for the Pokitto port I was working on:


The reason random exists in the first place is because the C spec requires that rand() returns an int, which is 16-bits on AVR,
so random() exists as a means of getting a 32-bit random number.
(Though technically only 31 of the bits are random.)

On platforms like SAMD, int is already 32 bit, so it makes sense that random wouldn’t exist.
(Note: random is a non-standard extension specific to avr-libc.)

1 Like

Yes, here’s the version I adapted for the SAMD fork:

unsigned long Arduboy2Base::generateRandomSeed()
{
  unsigned long seed;

#ifndef ARDUBOY_SAMD
  power_adc_enable(); // ADC on

  // do an ADC read from an unconnected input pin
  ADCSRA |= _BV(ADSC); // start conversion (ADMUX has been pre-set in boot())
  while (bit_is_set(ADCSRA, ADSC)) { } // wait for conversion complete

  seed = ((unsigned long)ADC << 16) + micros();

  power_adc_disable(); // ADC off
#else
	seed = (unsigned long)analogRead(A4) * ~micros() + micros();
#endif // ndef ARDUBOY_SAMD
  return seed;
}

Thanks - I went to WMath.cpp and WMath.h and added a parameterless version:

extern "C" {
  #include "stdlib.h"
  #include "stdint.h"
}
#include "WMath.h"

extern void randomSeed( uint32_t dwSeed )
{
  if ( dwSeed != 0 )
  {
    srand( dwSeed ) ;
  }
}

extern long random()
{
  return rand() % RANDOM_MAX;
}

extern long random( long howbig )
{
  if ( howbig == 0 )
  {
    return 0 ;
  }

  return rand() % howbig;
}

and it got waaay farther through compilation. Stopped here though:

C:\Users\...\AppData\Local\Temp\arduino_build_910817\sketch\SaveCheckState.cpp.o: In function `SaveCheckState::activate(GameStateMachine<GameContext, GameStateType>&)':
C:\Users\...\AppData\Local\Temp\arduino_build_910817\sketch/SaveCheckState.cpp:32: undefined reference to `__brkval'
C:\Users\...\AppData\Local\Temp\arduino_build_910817\sketch/SaveCheckState.cpp:32: undefined reference to `__heap_start'

Edit. I see @Pharap you have previous in this area…

1 Like

I’d completely forgot about that.
The annoying thing is I was intending to replace that with a clever alternative but never got round to it, and I’m not really in a position to edit and test it at the moment.

Basically the problem is that I was trying to be clever by actually measuring how much stack space is available (see getAvailableStackSpace) to prevent a stack overflow, but as it turns out variable-length arrays aren’t standard C++ anyway.

The clever alternative I haven’t got around to implementing is to use the Arduboy’s 1KB screen buffer as scratch space instead of allocating a new buffer on the stack.

If you want you can just forget Minesweeper for now.
Unfortunately I can’t think of any games that don’t have sound.

Are you using a vendor-supplied version of Arduino or one you’ve made yourself?

Assuming for a moment you did write the rest of the code there…

I’m not sure these externs should be here.
Did you mean to do extern "C" void randomSeed( uint32_t dwSeed ) et cetera?

extern means “this is defined in another translation unit”,
but you’re defining them at the point you’re declaring them.
extern "C" means “export this function using C-style linkage” (i.e. “do not mangle the name of this function”).

Come to think of it, I’m not sure you’re allowed to extern "C" overloaded functions.

Ah, I’d completely forgotten about that. Sure enough it’s no coincidence.

1 Like

In the hardware folder that contains e.g. boards.txt for custom boards, you can also keep versions of the Arduino libraries that are specific to that custom board, so I just changed the board-specific version of WMath.cpp etc. It won’t affect any other board I have and would be downloaded if anyone else were to “install” the same board in future.

I did not. The good folks at Arduino did!

I will see if I can track down the calls to getAvailableStackSpace and work/comment around it. Although I don’t really understand what the function is for at the moment, I’m assuming it’s a memory limitation/optimisation technique that isn’t going to be as important on the SAMD21 at this point.

The only downside to that approach is that if the ‘official’ version of a particular file changes you have to make sure to update your derived version.

The version on GitHub doesn’t appear to have those extra externs:

Is there a SAMD-specific version somewhere that I’m not aware of?

It’s not actually part of Arduino, it’s a local file in Minesweeper.
That thread you found was a discussion to try to get it added to Arduino’s API,
but like many feature request threads it was forgotten about.

As I said before, it was something I used to try to pre-empt and avoid a stack overflow situation.
It’s to prevent a memory-related error rather than to conserve memory.

It won’t really make sense unless you know what the call stack is.
When I have chance I’ll remove it from Minesweeper.

1 Like

But I think there is the basis of a workaround:

extern "C" char *sbrk(int i);

int FreeRam () {
  char stack_dummy = 0;
  return &stack_dummy - sbrk(0);
}

is claimed to be the SAMD M0 and M4 version. Source where I found it (they link to Arduino forum source). I’m just trying to work out how to merge it with ptrdiff_t and int vs char in the version we have in Minesweeper.

Hrm, strange. I might inquire about that sometime.
It’s particularly interesting that the file was like that when they first added it,
and there’s only one contributor listed.

Don’t worry about matching the exact behaviour,
this function only exists in Minesweeper, it’s not part of Arduino,
so it’s not worth porting.

For the sake of doing a test you can just make it return a reasonably large-ish constant value.
Something like 256 should be fine.
The SAMD has enough RAM that I’d be surprised if it didn’t have that much stack space spare.

#include <stddef.h>

inline ptrdiff_t getAvailableStackSpace()
{
	const char stackTop;
	
	return static_cast<ptrdiff_t>(&stackTop - sbrk(0));
}

If you have access to the C++ standard library rather than just C libraries, prefer:

#include <cstddef>

inline std::ptrdiff_t getAvailableStackSpace()
{
	const char stackTop;
	
	return static_cast<std::ptrdiff_t>(&stackTop - sbrk(0));
}

I doubt the char vs int will actually make a difference (I suspect the stack is word aligned), but I changed the type just in case it is relevant.

My instinct is that sbrk(0) should be equivalent to __brkval,
but I haven’t checked any documentation whatsoever,
I’m just assuming based on what I think it does.


A note about adafruit's article...

You can verify where data is stored by printing out the address:
Serial.print("Address of str $"); Serial.println((int)&str, HEX);

Please do not follow adafruit’s example of (int)&str.

If you must convert an address to an integer,
use intptr_t or uintptr_t from <stdint.h> (or better yet, std::intptr_t/std::uintptr_t from <cstdint> if you have access to those).

Casting a pointer to int is not portable,
there are platforms where int is not large enough to store the entire value of a pointer.
Ironically this is true of most x86-64/desktop environments,
where pointers are usually 64 bits large and int is usually 32 bits.

Also, prefer C++-style casting over C-style casting.
So (int)&str should ideally be something like reinterpret_cast<std::uintptr_t>(&string).

1 Like

Slight edit and it seems to have compiled:

extern "C" char *sbrk(int i);

inline ptrdiff_t getAvailableStackSpace()
{
	const char stackTop = 0;
	
	return static_cast<ptrdiff_t>(&stackTop - sbrk(0));
}

Ardugirl lives! Thanks @Pharap @MLXXXp and others for all your help so far. I hope this endeavour will be positive for the Arduboy community.

3 Likes

I’m currently in the process of getting rid of getAvailableStackSpace.
As expected though, it devolved into error spaghetti and the initial try doesn’t seem to be working properly,
so I’ve got quite a bit of debugging ahead of me.

For some reason the compiler was perfectly happy with the illegal use of reinterpret_cast in a constexpr context (a problem I knew about and have been intending to fix) until I started messing around with the functions to implement the ‘use the screen buffer as a checksum buffer’ thing.


I must admit I’m still not sold on that name.
Partly because it’s already being used for another project:
https://www.notsonoisy.com/ardugirl/
(This device might also interest @bateske.)
Partly because it’s so obvious it’s practically a cliché.

Naming is ultimately less important than getting it working though, that debate can wait.


While I think of it, have you tried inverting the screen colour?

The way LCDs work, lit pixels end up black and unlit pixels end up light,
which is the opposite of what the Arduboy does,
so the graphics look rather odd as a result.
Inverting the colours (with that command I mentioned the other day) should make things look more normal.