Undeclared identifiers digitalPinToPort and digitalPinToBitMask

[jgfoster]

When adding Arduino CI to Adafruit_BusIO we get the following output:

Located Arduino binary...                              /Applications/Arduino.app
The set of compilers (1) isn't empty...                                        ✓
Checking g++ version... 
    Configured with: --prefix=/Applications/Xcode.app/Contents/Developer/usr --with-gxx-include-dir=/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/c++/4.2.1
    Apple clang version 12.0.0 (clang-1200.0.32.21)
    Target: x86_64-apple-darwin19.6.0
    Thread model: posix
    InstalledDir: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin
...Checking g++ version                                                        ✓
libasan availability for g++...                                             true
Requested unittest platform 'mega2560' is defined in 'platforms' YML...        ✓
Library conforms to Arduino library specification...                         1.5
Unit testing test.cpp with g++... 

Last command:  $ g++ -std=c++0x -o /Users/jfoster/Documents/Arduino/libraries/Adafruit_BusIO/unittest_test.cpp.bin -DARDUINO=100 -D__AVR__ -g -O1 -fno-omit-frame-pointer -fno-optimize-sibling-calls -fsanitize=address -D__AVR_ATmega2560__ -DARDUINO_CI -I/Users/jfoster/Documents/Arduino/libraries/Adafruit_BusIO/vendor/bundle/ruby/2.6.0/bundler/gems/arduino_ci-334b7aa377c8/cpp/arduino -I/Users/jfoster/Documents/Arduino/libraries/Adafruit_BusIO/vendor/bundle/ruby/2.6.0/bundler/gems/arduino_ci-334b7aa377c8/cpp/unittest -I/Users/jfoster/Documents/Arduino/libraries/Adafruit_BusIO/src /Users/jfoster/Documents/Arduino/libraries/Adafruit_BusIO/vendor/bundle/ruby/2.6.0/bundler/gems/arduino_ci-334b7aa377c8/cpp/arduino/Arduino.cpp /Users/jfoster/Documents/Arduino/libraries/Adafruit_BusIO/vendor/bundle/ruby/2.6.0/bundler/gems/arduino_ci-334b7aa377c8/cpp/arduino/Godmode.cpp /Users/jfoster/Documents/Arduino/libraries/Adafruit_BusIO/vendor/bundle/ruby/2.6.0/bundler/gems/arduino_ci-334b7aa377c8/cpp/arduino/stdlib.cpp /Users/jfoster/Documents/Arduino/libraries/Adafruit_BusIO/vendor/bundle/ruby/2.6.0/bundler/gems/arduino_ci-334b7aa377c8/cpp/unittest/ArduinoUnitTests.cpp /Users/jfoster/Documents/Arduino/libraries/Adafruit_BusIO/src/Adafruit_BusIO_Register.cpp /Users/jfoster/Documents/Arduino/libraries/Adafruit_BusIO/src/Adafruit_I2CDevice.cpp /Users/jfoster/Documents/Arduino/libraries/Adafruit_BusIO/src/Adafruit_SPIDevice.cpp /Users/jfoster/Documents/Arduino/libraries/Adafruit_BusIO/test/test.cpp

/Users/jfoster/Documents/Arduino/libraries/Adafruit_BusIO/src/Adafruit_SPIDevice.cpp:51:48: error: use of undeclared identifier 'digitalPinToPort'
  csPort = (BusIO_PortReg *)portOutputRegister(digitalPinToPort(cspin));
                                               ^
/Users/jfoster/Documents/Arduino/libraries/Adafruit_BusIO/src/Adafruit_SPIDevice.cpp:52:15: error: use of undeclared identifier 'digitalPinToBitMask'
  csPinMask = digitalPinToBitMask(cspin);
              ^
/Users/jfoster/Documents/Arduino/libraries/Adafruit_BusIO/src/Adafruit_SPIDevice.cpp:54:52: error: use of undeclared identifier 'digitalPinToPort'
    mosiPort = (BusIO_PortReg *)portOutputRegister(digitalPinToPort(mosipin));
                                                   ^
/Users/jfoster/Documents/Arduino/libraries/Adafruit_BusIO/src/Adafruit_SPIDevice.cpp:55:19: error: use of undeclared identifier 'digitalPinToBitMask'
    mosiPinMask = digitalPinToBitMask(mosipin);
                  ^
/Users/jfoster/Documents/Arduino/libraries/Adafruit_BusIO/src/Adafruit_SPIDevice.cpp:58:51: error: use of undeclared identifier 'digitalPinToPort'
    misoPort = (BusIO_PortReg *)portInputRegister(digitalPinToPort(misopin));
                                                  ^
/Users/jfoster/Documents/Arduino/libraries/Adafruit_BusIO/src/Adafruit_SPIDevice.cpp:59:19: error: use of undeclared identifier 'digitalPinToBitMask'
    misoPinMask = digitalPinToBitMask(misopin);
                  ^
/Users/jfoster/Documents/Arduino/libraries/Adafruit_BusIO/src/Adafruit_SPIDevice.cpp:61:49: error: use of undeclared identifier 'digitalPinToPort'
  clkPort = (BusIO_PortReg *)portOutputRegister(digitalPinToPort(sckpin));
                                                ^
/Users/jfoster/Documents/Arduino/libraries/Adafruit_BusIO/src/Adafruit_SPIDevice.cpp:62:16: error: use of undeclared identifier 'digitalPinToBitMask'
  clkPinMask = digitalPinToBitMask(sckpin);
               ^
8 errors generated.
...Unit testing test.cpp with g++                                              ✗
Skipping compilation of examples...                as requested via command line
Failures: 1

Are these things that should be part of the mock Arduino?

[ianfixes]

Do you have any test cases for these? I'm wondering if it will be as simple as what's in https://github.com/Arduino-CI/arduino_ci/blob/master/cpp/arduino/Arduino.h#L66

#define analogInPinToBit(P) (P)
#define digitalPinToInterrupt(P) (P)

[jgfoster]

The test script is here.

I tried adding the following to the top of Adafruit_SPIDevice.cpp:

#define digitalPinToBitMask(P) (P)
#define digitalPinToPort(P) (P)

And now we have the following:

/Users/jfoster/Documents/Arduino/libraries/Adafruit_BusIO/src/Adafruit_SPIDevice.cpp:54:29: error: use of undeclared identifier 'portOutputRegister'
  csPort = (BusIO_PortReg *)portOutputRegister(digitalPinToPort(cspin));
                            ^
/Users/jfoster/Documents/Arduino/libraries/Adafruit_BusIO/src/Adafruit_SPIDevice.cpp:57:33: error: use of undeclared identifier 'portOutputRegister'
    mosiPort = (BusIO_PortReg *)portOutputRegister(digitalPinToPort(mosipin));
                                ^
/Users/jfoster/Documents/Arduino/libraries/Adafruit_BusIO/src/Adafruit_SPIDevice.cpp:61:33: error: use of undeclared identifier 'portInputRegister'
    misoPort = (BusIO_PortReg *)portInputRegister(digitalPinToPort(misopin));
                                ^
/Users/jfoster/Documents/Arduino/libraries/Adafruit_BusIO/src/Adafruit_SPIDevice.cpp:64:30: error: use of undeclared identifier 'portOutputRegister'
  clkPort = (BusIO_PortReg *)portOutputRegister(digitalPinToPort(sckpin));

[ianfixes]

csPort = (BusIO_PortReg *)portOutputRegister(digitalPinToPort(cspin));

This is closer to what I mean in terms of use case -- I'm trying to figure out whether #define someMacro(P) (P) actually produces values that will work in a unit test environment. portInputRegister is definitely going to fail if I define it in that way, because it seems to be expected to return a pointer.

[matthijskooijman]

An alternative fix for this would be to fiddle with io.h so it actually declares all IO registers as meaningful variables (probably by indexing into a global array of all I/O registers) that can be read and written to. This will automatically make all code that uses AVR registers actually compile.

Then, the portOutputRegister and similar macros can be copied verbatim from the actual Arduino variant files, declaring them to actually point into the right registers.

In a later step, the mock pin code could maybe also look at those I/O registers so writes to them are actually processed.

[matthijskooijman]

Also, I believe these definitions should be guarded by __AVR__, since the interface it's trying to mock is specific to the AVR Arduino core. Other cores might have similar macros, but since this ties in with low-level hardware registers, they'll be different (even if not named differently, then the way they work will likely be different).

[ianfixes]

io.h is part of the biggest "liability" of the entire project... it's a set of header files adopted from the official IDE tarball, and I brute-forced my way through macro writing to get things to compile. It was ugly... and there was much gnashing of teeth moving things between my Arduino.h, the io.h, and GODMODE.

I have a feeling there's a better way to do a bunch of this stuff, but all I know about the Arduino codebase was gleaned by hacking my way through it for this project in particular and not by being an expert going into it. (I'm still not...)

Should we open a separate issue to track whatever #197 hasn't covered (to replace this issue), or continue the discussion here?

[jgfoster]

I believe these definitions should be guarded by __AVR__

That can certainly be done. As to the "else" portion, should I just leave them undefined since I'm not in a position to fully implement them for every architecture?

[jgfoster]

I've updated the code to be guarded by __AVR__ and took out the padding since it was an (incorrect) attempt to handle other boards. Otherwise, I haven't addressed the more elaborate solutions involving io.h. Perhaps the discussion should be moved to #197. Is that reasonable?