Issue with Arduboy FX Arduino Plugin and uint8_t

Dolorre · February 19, 2023, 3:15pm

I 'm having issues changing a statement to be used on the FX chip. I don’t know if I’m just ignorant, or what. @Mr.Blinky, any ideas?

Here’s a snippet of the original statement:


const uint8_t gti[] PROGMEM = { 255,255,16,66,79,79,77,33,32,82,85,77,66,76,69,33,32,70,76,65,83,72,33,0,255,255,13,1,255,255,13,0,255,255,16,65,110,32,101,120,112,108,111,115,105,111,110,32,115,111,109,101,119,104,101,114,101,32,119,105,116,104,105,110,32,153,};

When I try to take the statement below (also just a snippet) and put it into the fxdata.txt, build it, it works. But then when I go to export compiled binary, it throws errors.

uint8_t gti[] = { 
   255, 255, 16, 66, 79, 79, 77, 33, 32, 82, 85, 77, 66, 76, 69, 33, 32, 70, 76, 65, 83, 72, 33, 0, 255, 255, 13, 1, 255, 255, 13, 0, 255, 255, 16, 65, 110, 32, 101, 120, 112, 108, 111, 115, 105, 111, 110, 32, 115, 111, 109, 101, 119, 104, 101, 114, 101, 32, 119, 105, 116, 104, 105, 110, 32,153
 };

Here are the errors:

Arduino: 1.8.19 (Windows 10), Board: "Arduboy"

In file included from C:\Users\Dolorre\AppData\Local\Arduino15\packages\arduino\hardware\avr\1.8.6\cores\arduino/Arduino.h:28:0,

                 from sketch\Silence_FX.ino.cpp:1:

E:\Documents\Arduboy FX Games\Silence_FX\Silence_FX.ino: In function 'void startVM(uint16_t)':

Silence_FX:62:47: error: invalid types 'const uint24_t {aka const __uint24}[uint16_t {aka unsigned int}]' for array subscript

   uint16_t type = (pgm_read_byte_near(&(gti[pc])) << 8) | (pgm_read_byte_near(&(gti[pc+1])) & 0xFF);  // Read in type of frame

                                               ^

Silence_FX:62:89: error: invalid types 'const uint24_t {aka const __uint24}[uint16_t {aka unsigned int}]' for array subscript

   uint16_t type = (pgm_read_byte_near(&(gti[pc])) << 8) | (pgm_read_byte_near(&(gti[pc+1])) & 0xFF);  // Read in type of frame

                                                                                         ^

Silence_FX:67:48: error: invalid types 'const uint24_t {aka const __uint24}[uint16_t {aka unsigned int}]' for array subscript

      uint8_t mode = pgm_read_byte_near(&(gti[pc]));                                               // get the special packet mode

                                                ^

Silence_FX:76:56: error: invalid types 'const uint24_t {aka const __uint24}[uint16_t {aka unsigned int}]' for array subscript

              uint8_t buff = pgm_read_byte_near(&(gti[pc]));                                       // read a chracter

                                                        ^

Silence_FX:93:45: error: invalid types 'const uint24_t {aka const __uint24}[uint16_t {aka unsigned int}]' for array subscript

            pc = (pgm_read_byte_near(&(gti[pc])) << 8) | (pgm_read_byte_near(&(gti[pc+1])) & 0xFF);    // Set the program counter to the address in 16 bit field

                                             ^

Silence_FX:93:87: error: invalid types 'const uint24_t {aka const __uint24}[uint16_t {aka unsigned int}]' for array subscript

            pc = (pgm_read_byte_near(&(gti[pc])) << 8) | (pgm_read_byte_near(&(gti[pc+1])) & 0xFF);    // Set the program counter to the address in 16 bit field

                                                                                       ^

Silence_FX:101:49: error: invalid types 'const uint24_t {aka const __uint24}[uint16_t {aka unsigned int}]' for array subscript

             effect = pgm_read_byte_near(&(gti[pc]));                                              // Get effect type

                                                 ^

Silence_FX:141:56: error: invalid types 'const uint24_t {aka const __uint24}[uint16_t {aka unsigned int}]' for array subscript

              uint8_t buff = pgm_read_byte_near(&(gti[pc]));                                       // Read a character

                                                        ^

Silence_FX:159:56: error: invalid types 'const uint24_t {aka const __uint24}[uint16_t {aka unsigned int}]' for array subscript

       uint16_t alterexit = (pgm_read_byte_near(&(gti[pc])) << 8) | (pgm_read_byte_near(&(gti[pc+1])) & 0xFF); // get jump b address

                                                        ^

Silence_FX:159:98: error: invalid types 'const uint24_t {aka const __uint24}[uint16_t {aka unsigned int}]' for array subscript

       uint16_t alterexit = (pgm_read_byte_near(&(gti[pc])) << 8) | (pgm_read_byte_near(&(gti[pc+1])) & 0xFF); // get jump b address

                                                                                                  ^

Silence_FX:163:51: error: invalid types 'const uint24_t {aka const __uint24}[uint16_t {aka unsigned int}]' for array subscript

         uint8_t buff = pgm_read_byte_near(&(gti[pc]));                                                        // get a chracter

                                                   ^

Silence_FX:174:51: error: invalid types 'const uint24_t {aka const __uint24}[uint16_t {aka unsigned int}]' for array subscript

         uint8_t buff = pgm_read_byte_near(&(gti[pc]));                                                        // get a character

                                                   ^

Silence_FX:186:56: error: invalid types 'const uint24_t {aka const __uint24}[uint16_t {aka unsigned int}]' for array subscript

              uint8_t buff = pgm_read_byte_near(&(gti[pc]));                                                   // get character from main description

                                                        ^

exit status 1

invalid types 'const uint24_t {aka const __uint24}[uint16_t {aka unsigned int}]' for array subscript

Finally, here’s what in the fxdata.h file (which I have not touched):

#pragma once

/**** FX data header generated by fxdata-build.py tool version 1.14 ****/

using uint24_t = __uint24;

// Initialize FX hardware using  FX::begin(FX_DATA_PAGE); in the setup() function.

constexpr uint16_t FX_DATA_PAGE  = 0xffc2;
constexpr uint24_t FX_DATA_BYTES = 15833;

constexpr uint24_t gti = 0x000000;

Any idea if I’m being ignorant or if there’s something wrong?

Pharap · February 19, 2023, 7:28pm

You’re trying to use pgm_read_byte_near to read data from the FX chip.
pgm_read_byte & co are for reading data from internal progmem, not for reading data from the external FX chip.

You need to be using the FX library instead.

Dolorre · February 19, 2023, 7:48pm

Aha! So I was just being ignorant. Thanks @Pharap! I’ll take a look at the library.

filmote · February 19, 2023, 9:41pm

To read the first byte:

FX::seekData(gti);
uint8_t x = FX::readPendingUInt8();
FX::readEnd();

You can put the read within a loop to read all bytes or use FX::seekData(gti + pos); to position to a specific byte.

Dolorre · February 20, 2023, 2:07am

So… I’m using an array, would FX::seekDataArray() be better? If so, what parameters need to be passed to it? I don’t see any notes on it in the ArduboyFX.h file.

filmote · February 20, 2023, 2:27am

If you are reading a portion of the array, then you can use:

FX::seekDataArray(gti, index, offset, elementSize);

Assume your array was an 8x6 grid as shown below:

uint8_t gti[] = { 
1, 1, 1, 1, 1, 1, 1, 1,
2, 2, 2, 2, 2, 2, 2, 2,  
3, 3, 3, 3, 3, 3, 3, 3, 
4, 4, 4, 4, 4, 4, 4, 4, 
5, 5, 5, 5, 5, 5, 5, 5,
6, 6, 6, 6, 6, 6, 6, 6,
};

To read the row with all 4’s:

uint8_t data[8];

FX::seekDataArray(gti, 3, 0, 8);
for (uint8_t x = 0; x < 8; x++) {
  data[x] = FX::readPendingUInt8();
}
FX::readEnd();

Index (essentially row) = 3 as its zero-based, elementSize (length of each row) = 8. Offset can be used to read partial rows.

You could speed up the process by ditching the loop and doing this instead:

uint8_t data[8];

FX::seekDataArray(gti, 3, 0, 8);
FX::readBytes((uint8_t*)&data, sizeof(data));
FX::readEnd();

HTH

Pharap · February 20, 2023, 9:54am

I really need to find some time to submit some template functions to this library.
That could easily be FX::read(data).

filmote · February 20, 2023, 10:04am

I saw your templated version of this earlier and was wondering if @Mr.Blinky had implemented it yet. Its not the end of the world putting the structure and its size but it would be nicer / easier to have it ‘hidden’.

Mr.Blinky · February 20, 2023, 10:37am

I’ve implemented the templates for the load and saveGameState functions but there’s no update yet as I’m still tweaking some other things.

Pharap · February 20, 2023, 10:55am

I just started writing some template functions so there’s a chance that what I’m doing might end up clashing with some of your changes.

This is what I’ve got so far:

Specifically:

FX::readJedecID(id)
- (Uses a reference instead of a pointer, thus eliminating the risk of passing a null pointer and sparing people from the extra &.)
FX::seekArrayElement<Type>(address, index)
FX::seekArrayElementMember<Type>(address, index, offset)
FX::readObject(object)
FX::readObject(address, object)

All of which I’ve given proper Doxygen documentation.

I may be calling the wrong functions in places though, I’m not entirely sure what the ‘end’ variants actually do since what ‘ends the read command’ isn’t documented anywhere as far as I could see.

(There’s a few other changes I’d like to suggest, but they’d mainly be breaking changes, so I’m holding off on those.)

Aside from convinience there’s two other major benefits:

Because the library is doing the work for you, it’s much harder to get it wrong, which saves time hunting bugs introduced by accident.
It’s a lot friendlier for beginners to not have to see or know about casting and sizeof, particularly because explaining why they’re needed would take a lot of explaining.

Mr.Blinky · February 20, 2023, 12:44pm

Looking good. Feel free to do a PR.

Something I need to work on

Think of them of closing the stream that was opened by the seek commands.

Pharap · February 20, 2023, 1:31pm

I already had one ready about 30 minutes before your reply:

github.com/MrBlinky/ArduboyFX

Add Various Template and Reference-Parameter Functions

MrBlinky:master ← Pharap:development

opened 12:13PM - 20 Feb 23 UTC

Pharap

+121 -2

Adds: * `readJedecID(JedecID & id)` - Same as `readJedecID(JedecID * id), but u…ses a reference instead and is thus never `nullptr`. * `seekArrayElement<Type>(uint24_t address, uint8_t index)` - Seeks an array element. * Performs a `static_assert` to ensure that `sizeof(Type) > 0`, though by the laws of C++ that expression should _always_ be true. * Implemented manually rather than deferring to `seekDataArray` because `seekDataArray` manually handles the case of size being `0` at runtime and this version instead ensures that size is never `0` at compile time, thus this should be capable of producing smaller code. * `seekArrayElementMember<Type>(uint24_t address, uint8_t index, size_t offset)` - Seeks a member of an array element. * I'm presuming this is the sort of thing `seekDataArray`'s `offset` parameter is indended to do. I made this a separate function from `seekArrayElement` because seeking a specific member of an object in an array seems like a very niche thing to do, and is liable to confuse people who are only looking to read a single element from an array. The majority of users are going to want the `seekArrayElement` version. * `readObject<Type>(Type & object)` - A thin, type-safe wrapper around `readBytes`. * Absolves the user of having to deal with casting, `&` or `sizeof`, and thus prevents errors that would arise from manually dealing with boilerplate. * `readDataObject<Type>(uint24_t address, Type & object)` - A thin, type-safe wrapper around `readDataBytes`. * The documenation is probably off because I basically had to guess at what `readDataBytes` does. * `readSaveObject<Type>(uint24_t address, Type & object)` - A thin, type-safe wrapper around `readSaveBytes`. * Again, the documenation is probably off because I basically had to guess at what `readSaveBytes` does. * `loadObject<Type>(Type & object)` - A thin, type-safe wrapper around `loadGameState` * I opted for `Object` rather than `GameState` because it can be used for more than just games and because the words 'game state' get used (and 'overloaded') quite a lot in game development so giving them yet another meaning is likely to cause more confusion for beginners. * Again, absolves the user of having to deal with casting, `&` or `sizeof`, and thus prevents errors that would arise from manually dealing with boilerplate. * `saveObject<Type>(const Type & object)` - A thin, type-safe wrapper around `saveGameState` I was going to make a wrapper for `readDataArray`, but I was really confused about what it was supposed to be doing. I also wondered if `writeSavePage` might benefit from being templated, but I don't know enough about what it's actually supposed to be doing to be able to properly document a wrapper for it.

If you want to debate the naming choices or provide more information about readDataArray or writeSavePage, feel free.

I also made some issues outlining the other thoughts I had about things that could be improved.

Yeah, documentation was one of the issues I raised.

I don’t mind helping out with that, but there’s only a handful of things that I actually still remember/understand.

So is it worth e.g. adding a readObjectEnd function to wrap readBytesEnd?

Mr.Blinky · February 20, 2023, 1:57pm

I see. I’m not at home and not any getting notifications.

I’ll have a look at them.

Not really. If no further data needs to be read disable will suffice. If there’s a final byte to be read and you want to ‘close the stream’ then readEnd() wis used.

Pharap · February 20, 2023, 2:17pm

That wasn’t an admonishment for not checking, that was just to let you know it’s there.

(I probably check my GitHub notifications far less than you do.)

Anyway, there’s no rush. I just wanted to get the code done and my ideas documented while I actually had the time to spare.

Then readBytesEnd is also unnecessary?

Mr.Blinky · February 20, 2023, 2:55pm

Oh forgot about that one.

Pharap · February 20, 2023, 4:44pm

That’s the one I was actually interested in.

To put it more plainly, at the moment there is this:

template<typename Type>
static void readObject(Type & object)
{
  readBytes(reinterpret_cast<uint8_t *>(&object), sizeof(object));
}

And I’m asking if it’s worth adding this:

template<typename Type>
static void readObjectEnd(Type & object)
{
  readBytesEnd(reinterpret_cast<uint8_t *>(&object), sizeof(object));
}

I.e. whether there’s actually a valid use-case for that. I’m presuming there is, but I wasn’t certain enough to add it without asking.

Dolorre · February 20, 2023, 8:04pm

I ended up going with @filmote’s first suggestion:

  FX::seekData(gti + pc);
  bt = FX::readPendingUInt8();
  FX::readEnd();

I think it’s going to work with the code I’ve implemented. At the least, it’s not throwing any errors.

However, I can’t get the screen to refresh to test it in the arduboy_sim.

So, how would I replace this code for use with the FX chip:

arduboy.clear();
arduboy.display();

Right now I have it as:

      FX::display(CLEAR_BUFFER);
      FX::display();

What should it be instead?

Ard_Flamingo · February 20, 2023, 8:22pm

It should just be FX::display(CLEAR_BUFFER) at the end/bottom of loop()

(this time my answer should actually work lol)

Dolorre · February 20, 2023, 9:04pm

Nope, doesn’t work. Maybe because loop() only calls startVM?

I need to play around a bit more so I can ask better questions.

Ard_Flamingo · February 20, 2023, 9:05pm

Weird. By loop() we are talking about the Arduino loop function, right?

Secret 20% off Arduboy Coupon!

Use coupon code "ERRORCOUPON" at checkout!

Buy Arduboy Now!

Issue with Arduboy FX Arduino Plugin and uint8_t