Create 16/32-bit compatible AVR Registers mocks, _SFR_MEM8 in particular

[ianfixes]

Feature Request

See discussion here: #115 (comment)

It's time to take a more formal approach to the issue of AVR register mocks. All options will be on the table including refactoring/renaming the files that are there already.

Figuring out where AdrAdc.{h,cpp} should live is minimum acceptance criteria for this issue.

[matthijskooijman]

The comment you linked suggests mocking all registers to a single memory location, but I'm not sure that's helpful, since it does not allow the tests to see read back any values written to the registers by the sketch or library code. Doing so could be quite helpful in testing in some cases. Also, this is actually already used in the SPI mock:
https://github.com/ianfixes/arduino_ci/blob/dac3560f340645df3fed0a71b94712c23547ccf1/cpp/arduino/SPI.h#L97-L103

I presume this reads back the SPI hardware register to see if the sketch/library wrote anything to it to configure the byte order, snice there is no official Arduino API to do so I think, so sketches probably do this manually if they need a non-default byte order. If all registers would map back to the same memory location, this code would not work as expected.

One approach I can imagine is to just allocate a block of memory to represent all of the registers, and just let the SFR_IOx macros refer to memory in that block. To figure out how many memory to allocate, I think the RAMSTART macro can be used (which is the address of the first RAM byte, everything before that is usually (or probably always) I/O registers on AVR.

This approach allows keeping the current approach (that I rather like) of including the iomxxx.h headers (as good as?) verbatim, and just fiddling with the surrounding defines. This could look like something like this (just a sketch, probably needs more details):

#define _AVR_IO_REG(type, mem_addr) (*(type*)(&avr_io_registers[mem_addr]))
#define __SFR_OFFSET 0x20
#define _SFR_IO8(io_addr) _AVR_IO_REG(uint8_t, io_addr + __SFR_OFFSET)
#define _SFR_IO16(io_addr) _AVR_IO_REG(uint16_t, io_addr + __SFR_OFFSET)
#define _SFR_MEM8(mem_addr) _AVR_IO_REG(uint8_t, mem_addr);
#define _SFR_MEM16(mem_addr) _AVR_IO_REG(uint16_t, mem_addr);
#define _SFR_MEM32(mem_addr) _AVR_IO_REG(uint32_t, mem_addr);

extern uint8_t avr_io_registers[RAMSTART];

Note the difference between SFR_IO and SFR_MEM: On AVR, the general purpose registers (r0, r1, etc.) are mapped at adress 0x0 - 0x20, the I/O registers are mapped starting at 0x20 (up to RAMSTART). All of these can be addressed using their memory address, but the first 64 of them can additionally be addressed using the in and out instructions. These instructions take the I/O address (so in 0 refers to the first I/O register, which is at memory address 0x20). For this reason, some registers are defined using _SFR_IO, passing the I/O address (0x0-0x3F, corresponding to memory address 0x20-0x5F), and the other registers are defined using _SFR_MEM, passing the memory address (so 0x60 and above). The above macros translate all the addresses into memory addresses, for simplicity (though this does leave 0x20 bytes unused at the bottom of avr_io_registers where the general purpose registers would normally have been mapped).

[matthijskooijman]

Note that the above is heavily based on what AVR-libc does itself in its sfr_defs.h file: https://github.com/vancegroup-mirrors/avr-libc/blob/master/avr-libc/include/avr/sfr_defs.h

[ianfixes]

Could we discuss this on #183 ? What I really need are some unit test cases in here:
https://github.com/Arduino-CI/arduino_ci/pull/183/files#diff-083797a134ebda0cf08560f7787a07d5360f0a701e4d949b8a2666fca4501a2fR11

Specifically, I'm looking for some reading & writing tests as a hint into whatever data structure is required to implement this.

[ianfixes]

I'm stuck because #define __SFR_OFFSET 0x20 just points to inaccessible memory, and segfaults if a "register" based on this macro is actually accessed.

// hardware mocks, consolidated from _AVR_IO_REG and __SFR_OFFSET
volatile long long __ARDUINO_CI_SFR_MOCK[1024];
#define _SFR_IO8(io_addr) (*(volatile uint8_t *)(__ARDUINO_CI_SFR_MOCK + io_addr))
#define _SFR_IO16(io_addr) (*(volatile uint16_t *)(__ARDUINO_CI_SFR_MOCK + io_addr))
#define _SFR_MEM8(io_addr) (*(volatile uint8_t *)(__ARDUINO_CI_SFR_MOCK + io_addr))
#define _SFR_MEM16(io_addr) (*(volatile uint16_t *)(__ARDUINO_CI_SFR_MOCK + io_addr))
#define _SFR_MEM32(io_addr) (*(volatile uint32_t *)(__ARDUINO_CI_SFR_MOCK + io_addr))

Depending on where I put this, I get

cpp/arduino/SPI.h:97:11: error: use of undeclared identifier '_SFR_IO8'
    if (!(SPCR & (1 << DORD))) {

or

SampleProjects/TestSomething/test/defines.cpp:19:3: warning: expression result unused [-Wunused-value]
  &DDRE;
  ^~~~~
1 warning generated.
duplicate symbol ___ARDUINO_CI_SFR_MOCK in:
    /var/folders/8q/v8rqv1l91rq9zxk34mg_rxx00000gn/T/Arduino-e26d2d.o
    /var/folders/8q/v8rqv1l91rq9zxk34mg_rxx00000gn/T/Godmode-de5ba4.o
duplicate symbol ___ARDUINO_CI_SFR_MOCK in:
    /var/folders/8q/v8rqv1l91rq9zxk34mg_rxx00000gn/T/Arduino-e26d2d.o
    /var/folders/8q/v8rqv1l91rq9zxk34mg_rxx00000gn/T/ArduinoUnitTests-ca06f2.o
duplicate symbol ___ARDUINO_CI_SFR_MOCK in:
    /var/folders/8q/v8rqv1l91rq9zxk34mg_rxx00000gn/T/Arduino-e26d2d.o
    /var/folders/8q/v8rqv1l91rq9zxk34mg_rxx00000gn/T/test-something-069485.o
duplicate symbol ___ARDUINO_CI_SFR_MOCK in:
    /var/folders/8q/v8rqv1l91rq9zxk34mg_rxx00000gn/T/Arduino-e26d2d.o
    /var/folders/8q/v8rqv1l91rq9zxk34mg_rxx00000gn/T/defines-a3f12c.o
ld: 4 duplicate symbols for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)

I'm sure there's something obvious I'm missing with regards to the way I'm doing #include lines, but it's been a while since I had to navigate linker errors.

[jgfoster]

The duplicate symbol issue is because you are defining a variable in a header. It should be declared in the header as extern and then defined once in a .cpp file.

[ianfixes]

I think this was fixed by #183 but if not please reopen

[matthijskooijman]

I think it is. Re-reading #120 (comment) I do see that I originally suggested to use a 0x20 offset for the IO macros (since those get passed IO addresses, not memory addresses), which is not merged now. However, I don't think this is really important, unless the same register is accessed both through an IO and MEM macro (which I expect not to be the case).

[ianfixes]

If it's trivial to fix, I'll take a PR. Otherwise, just an idea of a unit test that demonstrates the issue would be fine for me to make progress on