Porting Arduboy2 library to SAMD

My hope is that changes could be merged to provide a single library at some point.

Yeah too bad. I also had it refunded. I haven’t made up my mind yet if I’m gonna buy the SparkFun one or Adafruits itsybitsy M4 Express, or hack the Adafruit PyBadge LC

Can I ask you, as a connoisseur of hardware, what screen would you pair with the itsybitsy m4? I’ve used one to develop for work and they’re nice.

Not fully settled yet if it will be 128x128 OLED with 4-bit greys or 160x128 Color OLED.

2 Likes

That was my plan when I modified the Arduboy library. I wanted you to be able to select the board type in the IDE and then be able to compile any sketch, written for the original Arduboy, for the new hardware, without even having to change the includes.

For places where there ends up too much #if interleaving, it could be broken up into separate library source files that are conditionally included.

1 Like

I had gathered as much, but the reason I find it notable in this case is because it raises the point of whether it’s worth the trouble - i.e. is it likely that @SimonMerrett’s SAMD port/adaptation will be merged with the original?

When I started my Pokitto port I started under the assumption that my changes wouldn’t make it back into the original, and thus the Pokitto port would exist as a standalone version.

I was about to suggest something along these lines.
I’ve had to do something similar in the past to account for compiler-specific attributes.
(Having standardised attribute syntax has made this much easier in recent years.)

One obvious candidate for separation is the pin definitions.
Having something along the lines of:

#if defined(AB_DEVKIT)
#include "Pins_Devkit.h"
#elif defined(ARDUBOY_10)
#include "Pins_Arduboy.h"
#elif defined(ARDUBOY_SAMD51)
#include "Pins_SAMD51.h"
#endif

Would save a lot of space in ArduboyCore.h.


But if something like this is the goal, then it’s also a chance to reevaluate how much of the API is platform dependent and question whether it’s worth maybe introducing some new functions and/or macros to account for platform specific behaviours and thus make it easier to implement new platforms.

E.g. should TXLED0 be wrapped in some kind of TXLedOff() macro/function which is to be implemented by each platform?

1 Like

That would be the plan, from my point of view. But it would only be merged back in if some kind of official (or at least standardised and popular) Arduboy successor, based on this arcitecture, came to light. Developing with that in mind is a good goal, though.

Those kind of optimizations could be ongoing.

2 Likes

Good. I’m happy for more experienced people to be the architects (for a grand title) and make librarian decisions. I’m just happy when I scroll down to the next compiler error and after half an hour manage to work out that Arduboy2Beep.cpp needs to #include Arduboy2.h if I’m going to splatter it with preprocessor macros.

1 Like

I think I you need a crash course in the C++ build process and what the words ‘translation unit’ mean. :P

1 Like

I think the 4 bit grayscale and the 128x128 display is best. There is also a 128x128 color oled too, and actually that one might even be cheapest, as it’s most available I think.

And all said and done, if people have a something that works and we want to try and do a campaign, I’m game, I’ll do my best to promote it but the goal of the campaign would have to be like $100k or that ballpark. From rough estimations.

That of course changes if it’s only the PCB. That’s easy. It’s the plastic parts that are tough.

2 Likes

Amen. The enclosure is critical for a “consumer” product, nigh on irrelevant for tinkerers/hackers and can be absolutely minimal for educational users (makers/players). The aliexpress knockoff kits are good examples of what could be perfectly acceptable to many people. There are also options for small lifespan moulds using aluminium or silicone that I’ve had very reasonable quotes for injection moulded parts in the past based on. I’m sure there are options in this space if people want to bring something to life at scales of 10+.

Thanks for your support, I realise that you have significant business pressures at the moment and hope they ease off soon.

Excellent - I was looking to see if I had put my Arduboy2.h #includes and #ifdefs in the wrong order but your helpful PM has cleared that up for me. Please point out other specific issues you see me creating, although I may struggle to digest all the terms in the links you sent me!

Could we add in the #ifdef ARDUBOY_SAMD to Sprites.h and use the alias conditionally? On my phone atm but will post code of what I mean in a while.

1 Like

The most important parts are to understand the preprocessing phase and realise that every .cpp file is a separate translation unit that only sees what it includes.

With those two things in mind it should be a lot easier to understand what order you need to define things in.

(I’d like to point out that actually #defines aren’t the best way of implementing most of these things,
but that’s what Arduboy2’s already using so that’s the approach we’re stuck with.)

#ifndef Sprites_h
#define Sprites_h

#include "Arduboy2Core.h"

#if defined(ARDUBOY_SAMD)
#include "SpritesB.h"

using Sprites = SpritesB;
#else
// Regular definition of Sprites
#endif

#endif

Although it might be better to do:

#ifndef Sprites_h
#define Sprites_h

#include "Arduboy2Core.h"

#if !defined(/*Some AVR specific macro*/)
#include "SpritesB.h"

using Sprites = SpritesB;
#else
// Regular definition of Sprites (using AVR assembly)
#endif

#endif

Strictly speaking SpritesB is actually the ‘base case’,
Sprites is the special platform-specific version,
so it makes sense to make the alias the default implementation,
and make the more detailed definition the ‘AVR only’ implementation.

1 Like

I’m trying to do the “better” version. A bit stuck. BTW, Sprites.h doesn’t #include "Arduboy2Core.h", in the version I have it #include "Arduboy2.h".

Here’s where I have got to in Sprites.h:

#ifndef Sprites_h
#define Sprites_h

#include "Arduboy2.h"
#include "SpritesCommon.h"

#if !defined(ARDUINO_ARCH_AVR)
	#include "SpritesB.h"

using Sprites = SpritesB;
#else

What confuses me is

because the assembly is in Sprites.cpp. Or do we just put the function declarations next in Sprites.h and the double #endif right at the bottom, like

/**
 * @file Sprites.h
 * \brief
 * A class for drawing animated sprites from image and mask bitmaps.
 */

#ifndef Sprites_h
#define Sprites_h

#include "Arduboy2.h"
#include "SpritesCommon.h"

// TODO: this section is experimental to address the issue of assembly being used in Sprites.cpp
#if !defined(ARDUINO_ARCH_AVR)
#include "SpritesB.h"

using Sprites = SpritesB;
#else
//end of experimental section - need to remove the additional #endif at the end of the file below if removing the experimental secion.

/** \brief
 * A class for drawing animated sprites from image and mask bitmaps.
 *
 * \details
 * The functions in this class will draw to the screen buffer an image
 * contained in an array located in program memory. A mask can also be
 * specified or implied, which dictates how existing pixels in the buffer,
 * within the image boundaries, will be affected.
 *
 * A sprite or mask array contains one or more "frames". Each frame is intended
 * to show whatever the sprite represents in a different position, such as the
 * various poses for a running or jumping character. By specifying a different
 * frame each time the sprite is drawn, it can be animated.
 *
 * Each image array begins with values for the width and height of the sprite,
 * in pixels. The width can be any value. The height must be a multiple of
 * 8 pixels, but with proper masking, a sprite of any height can be created.
 *
 * For a separate mask array, as is used with `drawExternalMask()`, the width
 * and height are not included but must contain data of the same dimensions
 * as the corresponding image array.
 *
 * Following the width and height values for an image array, or from the
 * beginning of a separate mask array, the array contains the image and/or
 * mask data for each frame. Each byte represents a vertical column of 8 pixels
 * with the least significant bit (bit 0) at the top. The bytes are drawn as
 * 8 pixel high rows from left to right, top to bottom. When the end of a row
 * is reached, as specified by the width value, the next byte in the array will
 * be the start of the next row.
 *
 * Data for each frame after the first one immediately follows the previous
 * frame. Frame numbers start at 0.
 *
 * \note
 * \parblock
 * A separate `SpritesB` class is available as an alternative to this class.
 * The only difference is that the `SpritesB` class is optimized for small
 * code size rather than for execution speed. One or the other can be used
 * depending on whether size or speed is more important.
 *
 * Even if the speed is acceptable when using `SpritesB`, you should still try
 * using `Sprites`. In some cases `Sprites` will produce less code than
 * `SpritesB`, notably when only one of the functions is used.
 *
 * You can easily switch between using the `Sprites` class or the `SpritesB`
 * class by using one or the other to create an object instance:
 *
 * \code{.cpp}
 * Sprites sprites;  // Use this to optimize for execution speed
 * SpritesB sprites; // Use this to (likely) optimize for code size
 * \endcode
 * \endparblock
 *
 * \note
 * \parblock
 * In the example patterns given in each Sprites function description,
 * a # character represents a bit set to 1 and
 * a - character represents a bit set to 0.
 * \endparblock
 *
 * \see SpritesB
 */
class Sprites
{
  public:
    /** \brief
     * Draw a sprite using a separate image and mask array.
     *
     * \param x,y The coordinates of the top left pixel location.
     * \param bitmap A pointer to the array containing the image frames.
     * \param mask A pointer to the array containing the mask frames.
     * \param frame The frame number of the image to draw.
     * \param mask_frame The frame number for the mask to use (can be different
     * from the image frame number).
     *
     * \details
     * An array containing the image frames, and another array containing
     * corresponding mask frames, are used to draw a sprite.
     *
     * For the mask array, the width and height are not included but must
     * contain data of the same dimensions as the corresponding image array.
     *
     * Bits set to 1 in the mask indicate that the pixel will be set to the
     * value of the corresponding image bit. Bits set to 0 in the mask will be
     * left unchanged.
     *
     *     image  mask   before  after  (# = 1, - = 0)
     *
     *     -----  -###-  -----   -----
     *     --#--  #####  -----   --#--
     *     ##-##  ##-##  -----   ##-##
     *     --#--  #####  -----   --#--
     *     -----  -###-  -----   -----
     *
     *     image  mask   before  after
     *
     *     -----  -###-  #####   #---#
     *     --#--  #####  #####   --#--
     *     ##-##  #####  #####   ##-##
     *     --#--  #####  #####   --#--
     *     -----  -###-  #####   #---#
     */
    static void drawExternalMask(int16_t x, int16_t y, const uint8_t *bitmap,
                                 const uint8_t *mask, uint8_t frame, uint8_t mask_frame);

    /** \brief
     * Draw a sprite using an array containing both image and mask values.
     *
     * \param x,y The coordinates of the top left pixel location.
     * \param bitmap A pointer to the array containing the image/mask frames.
     * \param frame The frame number of the image to draw.
     *
     * \details
     * An array containing combined image and mask data is used to draw a
     * sprite. Bytes are given in pairs with the first byte representing the
     * image pixels and the second byte specifying the corresponding mask.
     * The width given in the array still specifies the image width, so each
     * row of image and mask bytes will be twice the width value.
     *
     * Bits set to 1 in the mask indicate that the pixel will be set to the
     * value of the corresponding image bit. Bits set to 0 in the mask will be
     * left unchanged.
     *
     *     image  mask   before  after  (# = 1, - = 0)
     *
     *     -----  -###-  -----   -----
     *     --#--  #####  -----   --#--
     *     ##-##  ##-##  -----   ##-##
     *     --#--  #####  -----   --#--
     *     -----  -###-  -----   -----
     *
     *     image  mask   before  after
     *
     *     -----  -###-  #####   #---#
     *     --#--  #####  #####   --#--
     *     ##-##  #####  #####   ##-##
     *     --#--  #####  #####   --#--
     *     -----  -###-  #####   #---#
     */
    static void drawPlusMask(int16_t x, int16_t y, const uint8_t *bitmap, uint8_t frame);

    /** \brief
     * Draw a sprite by replacing the existing content completely.
     *
     * \param x,y The coordinates of the top left pixel location.
     * \param bitmap A pointer to the array containing the image frames.
     * \param frame The frame number of the image to draw.
     *
     * \details
     * A sprite is drawn by overwriting the pixels in the buffer with the data
     * from the specified frame in the array. No masking is done. A bit set
     * to 1 in the frame will set the pixel to 1 in the buffer, and a 0 in the
     * array will set a 0 in the buffer.
     *
     *     image  before  after  (# = 1, - = 0)
     *
     *     -----  -----   -----
     *     --#--  -----   --#--
     *     ##-##  -----   ##-##
     *     --#--  -----   --#--
     *     -----  -----   -----
     *
     *     image  before  after
     *
     *     -----  #####   -----
     *     --#--  #####   --#--
     *     ##-##  #####   ##-##
     *     --#--  #####   --#--
     *     -----  #####   -----
     */
    static void drawOverwrite(int16_t x, int16_t y, const uint8_t *bitmap, uint8_t frame);

    /** \brief
     * "Erase" a sprite.
     *
     * \param x,y The coordinates of the top left pixel location.
     * \param bitmap A pointer to the array containing the image frames.
     * \param frame The frame number of the image to erase.
     *
     * \details
     * The data from the specified frame in the array is used to erase a
     * sprite. To "erase" a sprite, bits set to 1 in the frame will set the
     * corresponding pixel in the buffer to 0. Frame bits set to 0 will remain
     * unchanged in the buffer.
     *
     *     image  before  after  (# = 1, - = 0)
     *
     *     -----  -----   -----
     *     --#--  -----   -----
     *     ##-##  -----   -----
     *     --#--  -----   -----
     *     -----  -----   -----
     *
     *     image  before  after
     *
     *     -----  #####   #####
     *     --#--  #####   ##-##
     *     ##-##  #####   --#--
     *     --#--  #####   ##-##
     *     -----  #####   #####
     */
    static void drawErase(int16_t x, int16_t y, const uint8_t *bitmap, uint8_t frame);

    /** \brief
     * Draw a sprite using only the bits set to 1.
     *
     * \param x,y The coordinates of the top left pixel location.
     * \param bitmap A pointer to the array containing the image frames.
     * \param frame The frame number of the image to draw.
     *
     * \details
     * Bits set to 1 in the frame will be used to draw the sprite by setting
     * the corresponding pixel in the buffer to 1. Bits set to 0 in the frame
     * will remain unchanged in the buffer.
     *
     *     image  before  after  (# = 1, - = 0)
     *
     *     -----  -----   -----
     *     --#--  -----   --#--
     *     ##-##  -----   ##-##
     *     --#--  -----   --#--
     *     -----  -----   -----
     *
     *     image  before  after
     *
     *     -----  #####   #####  (no change because all pixels were
     *     --#--  #####   #####  already white)
     *     ##-##  #####   #####
     *     --#--  #####   #####
     *     -----  #####   #####
     */
    static void drawSelfMasked(int16_t x, int16_t y, const uint8_t *bitmap, uint8_t frame);

    // Master function. Needs to be abstracted into separate function for
    // every render type.
    // (Not officially part of the API)
    static void draw(int16_t x, int16_t y,
                     const uint8_t *bitmap, uint8_t frame,
                     const uint8_t *mask, uint8_t sprite_frame,
                     uint8_t drawMode);

    // (Not officially part of the API)
    static void drawBitmap(int16_t x, int16_t y,
                           const uint8_t *bitmap, const uint8_t *mask,
                           uint8_t w, uint8_t h, uint8_t draw_mode);
};

#endif // experimental section
#endif

But when I tried that I get:

In file included from C:\Users\...\libraries\Arduboy2\src\Arduboy2.h:14:0,

                 from C:\Users\...\libraries\Arduboy2\src\SpritesB.h:11,

                 from C:\Users\...\libraries\Arduboy2\src\SpritesB.cpp:8:

C:\Users\...\libraries\Arduboy2\src\Sprites.h:17:17: error: expected type-specifier before 'SpritesB'

 using Sprites = SpritesB;

                 ^

C:\Users\...\libraries\Arduboy2\src\Sprites.cpp: In static member function 'static void SpritesB::drawBitmap(int16_t, int16_t, const uint8_t*, const uint8_t*, uint8_t, uint8_t, uint8_t)':

C:\Users\...\libraries\Arduboy2\src\Sprites.cpp:357:8: error: impossible constraint in 'asm'

       );

        ^

One of the problems I have is that I don’t know what functions get used most frequently, so I’m trying to get through the compiler with a basic sketch (the buttons example in the Arduboy2 library) first. I’d happily take advice on what to focus on.

Not necessarily stuck with. If someone wants to submit a PR with any possible #defines changed to constexpr equivalents or whatever else is appropriate, I’d consider accepting it. But it would have to:

  • Be all or nothing. I would want consistency. I wouldn’t want only some of the #defines addressed and others unchanged.

  • Have had adequate testing to be highly certain that the changes wouldn’t break or alter the behaviour of any existing sketches.

So, in order to move on, I followed @Pharap’s example from the Pokkito port and commented out all the asm in Sprites.cpp. The compiler then stopped complaining about everything and uploaded my test sketches, which are the HelloWorld and Buttons examples from the Arduboy2 library.

Neither worked, so I’m commenting out sections of the boot process to see if I can get buttons working using the onboard LED as my output for now. UPDATE - that works now I have ported the buttonState code from ArduboyZ rather than using my own

UPDATE - can’t see SPI SCK or MOSI on any of the pins, so it seems as if I haven’t managed to get the SPI peripheral working. Will go back and look at that in a bit

UPDATE - now got SPI commands initialising the OLED but a complete “boot logo” won’t work atm. It’s because I forgot to put SPI.begin(); anywhere in the boot process :roll_eyes:

Just as a more suitable place to record our thoughts about where the SAMD port of the library should be aiming for, I started a SAMD_ROADMAP.md in my fork. I haven’t managed to scrape everything we’ve covered here into it but will and am obviously open to PRs.

I’ve said it before but my reason for switching to a different microcontroller would be to address primarily one issue: Running out of memory.

I know some people like the challenge of optimising their code so that their concept can be squeezed into the resource limits of the ATmega32U4 but for many others it’s just a source frustration and the need to spend (or waste) extra time. Otherwise, the Arduboy is an easy and fun way of getting into the world of programming, with very few direct rivals.

Other than changing the microcontroller, I would keep it as close as possible to the existing Arduboy.

  • The same OLED display, battery, buttons, piezo speaker, RGB LED.
  • The same case except possibly slightly modified to allow access to a Micro-SD slot.
  • The circuit board would be exactly the same form factor so all the above items could continue to be used. Pads for additional enhancements, such as IR communication or an accelerometer/gyroscope or expansion header, could be added if space allowed but wouldn’t necessarily have to be populated.

I think we should try to avoid “feature creep” to any great extent, especially if it impacts cost.

The idea is that virtually all existing sketches (including those that talk directly to the display) would run on the new hardware after just being recompiled. Likewise, almost all exiting guides and tutorials would still apply verbatim.

I know a lot of people desire a display with more pixels and/or grey scale or colour but unless this display was 100% backwards compatible, from a software command and speed (as in update and pixel blurring) perspective, and could fit in the existing case when attached to the new circuit board, it wouldn’t be suitable in my mind. Also, with such a display you start to compete with other higher end devices.

Now, what I’ve said here has some bearing on decisions for porting the library, but if we want to continue discussing or debating this, I think it should be continued in a new topic or one of the numerous existing ones.

3 Likes