Tympan / Tympan_Library

Arduino/Teensy Library for Tympan Open Source Hearing Aid
MIT License
116 stars 31 forks source link

Sketch compiles when Tympan object does not match Arduino IDE Board #43

Closed hgeithner-creare closed 2 years ago

hgeithner-creare commented 2 years ago

When testing the compatibility of Teensyduino 1.54 with Tympan Rev D and E I noticed that the user can successfully compile and upload a sketch using the wrong Tympan Rev. For example, I plug in a Rev D and set the Arduino IDE board to Teensy 3.6 but I do not change myTympan(TympanRev::E). The sketch compiles and uploads to the Tympan, but the hardware does not work correctly.

chipaudette commented 2 years ago

I thought that this was going to be an easy fix. I was going to have the C0preprocessor compare the CPU type (which maps to Teensy rev) to the TympanRev in the code. If the C-preprocessor were to find a discrepancy, it would throw an error and stop compiling. It was gonna be easy!

For example, depending upon what model of Teensy you select in the Arduino IDEA, it creates a #define that matches the CPU that's at the heart of the Teensy:

Then, in our library (such as in Tympan.h) we could create a test that would see if the TympanRev aligned with the specified CPU model. If the correct #define doesn't exist, we'd have the compiler throw an error. You can throw a compiler error simply by #error. For example:

#error You requested TympanRev::E but the Arduino IDE is not set for Teensy 4.1. Please choose Teensy 4.1.

Unfortunately, this simple idea won't work.

The problem is that the C-preprocessor doesn't know what TympanRev we have specified in the sketch itself. For example, by my design, this is a typical way of specifying the TympanRev:

Tympan myTympan(TympanRev::E);

Sadly, to my knowledge, nothing in this line of text is available to the C-preprocessor. Bascially, the C-preprocessor has no idea what TympanRev I am assuming. Therefore, the preprocessor cannot perform any test. :(

We could choose to use a #define to set our TympanRev. If so, the preprocessor could do the comparison and we'd be good. But, such an approach would require me to change all of the example sketches. There's no time for that. So, I'm going to put this new feature on ice for a while.

Unless, @ijenkins-creare, do you have any better ideas?

chipaudette commented 2 years ago

Since I can't figure out how to do it at compile time, I've added a check to Tympan.cpp that checks the Tympan Rev at runtime. It checks at startup. So, if you've got the SerialMonitor open when the Tympan is booting, you should see an error if you've specified the wrong Tympan Rev.

Specifically, it looks at the compiler flag set by the Arduino IDE for which Teensy CPU is being used. It compares this compiler flag against what TympanRev you specify in your sketch. If the two disagree, a big multi-line warning is printed to the SerialMonitor.

If you've specified the wrong TympanRev, there will likely be TONS of other stuff going wrong, too...so lots and lots more error messages are likely to scroll onto the screen. These errors will probably push this new warning off the top of the SerialMonitor window. But, if you scroll all the way back to the beginning, you should see the new multi-line warning about using the wrong TympanRev.

It might be better if it were a bit later in the startup process, as some people (apparently Haley is one such person?) whose computer is unhappy with the SerialMonitor and she has to quickly open it manually upon startup. She might miss such a warning.

For anyone who wishes to test it, this currently lives in the feature/baudrate branch. Sorry. It should work with any sketch that creates a Tympan. If the sketch is set for RevE, then set the Arduino IDE to compile for Teensy 3.6 (which is RevD) and you should get the WARNING as soon as the sketch starts to run.

hgeithner-creare commented 2 years ago

It might be better if it were a bit later in the startup process, as some people (apparently Haley is one such person?) whose computer is unhappy with the SerialMonitor and she has to quickly open it manually upon startup. She might miss such a warning.

All good on my end now. I realized I was selecting from "Serial Ports" instead of "Teensy Ports". I was doing this because I learned it from the documentation. I will update the image to show choosing the "Teensy Ports" instead of the "Serial Ports".