Pokitto IR project Code Review

Pokitto Programming C++ language
1

Hello!

Consider the rest of this article a Spoiler for the up coming Pokitto Magazine #2. So if you’d like to keep that a surprise, or don’t mind spoilers (like me) then please continue :slight_smile:

I am pretty new to C++. I’ve dabbled for a while but never really took on any actual completable project. Until now! (And, I dare say, much more in the future!)

The reason for this post is I’d like some a code review of my project before I write it up in article format forever (ominous hehe). Since I am new to C++, I’d like to make sure that the code is “clean” and suitable for new folks and experienced alike. So! That long-winded explanation out of the way, to the code!

First Draft 
#include "Pokitto.h"
#include <IRremote/IRremote.h>

IRsend irsend(EXT0); // PWM output for IR blaster
IRrecv irrecv(EXT3); //currently only 3+ works for the receiver. 

// Storage for the recorded code
decode_results results; // results of reading an incoming signal
int codeType = -1; // The type of code
unsigned int rawCodes[RAWBUF]; // The durations if raw
int codeLen; // The length of the code
int readmode = 0; //read mode toggle. 0 = false, 1 = true

using PC=Pokitto::Core;
using PD=Pokitto::Display;

// Stores the code for later playback
// Most of this code is just logging
void storeCode(decode_results *results) {
    PD::print("Received code, saving as raw\n");
    
    codeType = results->decode_type;
    
    codeLen = results->rawlen - 1;
    // To store raw codes:
    // Drop first value (gap)
    // Convert from ticks to microseconds
    for (int i = 1; i <= codeLen; i++) {
        rawCodes[i - 1] = results->rawbuf[i]*USECPERTICK;
        PD::print(rawCodes[i - 1]);
    }
    PD::print("");
}

int main(){
    
    PC::begin();
    PD::persistence=true;
    PD::invisiblecolor = 0;
    PD::setFont(font5x7);
    PD::print("PEX IR test\nPress B to scan or A to send.");
    PD::update();
    irrecv.enableIRIn();
    
    while( PC::isRunning() ){
        
        if(PC::buttons.bBtn()){
            PD::clear();
            PD::print("Entered scan mode...\n");
            PD::update();
            readmode = 1;
        }
        
        if(PC::buttons.aBtn()){
            PD::clear();
            PD::print("Sending signal\n");
            
            readmode = 0;
            int count = results.rawlen;
            for (int i = 1; i < count; i++) {
                if (i & 1) {
                    PD::print(results.rawbuf[i]*USECPERTICK);
                } else {
                    PD::print('-');
                    PD::print((unsigned long) results.rawbuf[i]*USECPERTICK);
                }
                PD::print(" ");
            }
            PD::update();
            
            irsend.sendRaw(rawCodes, codeLen, 38);
            
            PD::print("\nWaiting for signal to finish...");
            PD::update();
            wait(1);//Give time to send result without receiving at the same time
        } else if (irrecv.decode(&results) && readmode == 1) {
            PD::clear();
            PD::printf("IR Received type = %d, code = %x\n", results.decode_type, results.value);
            
            storeCode(&results);
            
            PD::update();//storeCode uses print()
            
            irrecv.resume(); // resume receiver
            
            PD::print("Waiting for input or command.");
            PD::update();
        }
    }
    
    return 0;
}

After some great help, I have a second draft for review! :smiley:

Second Draft 
#include <Pokitto.h>
#include <IRremote/IRremote.h>

// PWM output for IR blaster
IRsend irSender(EXT0);

// Currently only 3+ works for the receiver. 
IRrecv irReceiver(EXT3); 

// results of reading an incoming signal
decode_results results; 

// The durations if raw
unsigned int rawCodes[RAWBUF]; 

// The length of the code
int codeLen; 

enum class Mode
{
    Send,
    Scan,
};

Mode mode = Mode::Send;

// Stores the signal for later sending
void storeCode(decode_results results) 
{
    using Pokitto::Display;
    
    // Used again when sending the signal
    codeLen = results.rawlen - 1;
    
    // To store raw codes:
    // Drop first value (gap)
    // Convert from ticks to microseconds
    for (int i = 1; i <= codeLen; i++)
    {
        rawCodes[i - 1] = results.rawbuf[i] * USECPERTICK;
        
        Display::print(rawCodes[i - 1]);
        Display::print(" ");
    }
    Display::print("\n");
}

// Prints the code benig sent to the screen
void printSentCode()
{
    using Pokitto::Display;
    
    for (int i = 1; i < results.rawlen; i++) 
    {
        if ((i % 2) != 0) 
        {
            Display::print(results.rawbuf[i] * USECPERTICK);
        } 
        else
        {
            Display::print('-');
            Display::print((unsigned long) results.rawbuf[i] * USECPERTICK);
        }
        Display::print(" ");
    }
}

int main()
{
    using Pokitto::Core;
    using Pokitto::Display;
    
    Core::begin();
    
    Display::persistence = true;
    Display::invisiblecolor = 0;
    Display::setFont(font5x7);
    Display::print("PEX IR test\nPress B to scan or A to send.\nC to open this menu.");
    
    irReceiver.enableIRIn();
    
    while(Core::isRunning())
    {
        
        if (Core::buttons.cBtn())
        {
            Display::clear();
            Display::print("Press B to scan or A to send.");
            
            mode = Mode::Send;
        }
        
        if (Core::buttons.bBtn())
        {
            Display::clear();
            Display::print("Entered scan mode...\n");
            
            mode = Mode::Scan;
        }
        
        if (Core::buttons.aBtn())
        {
            Display::clear();
            Display::print("Sending signal:\n");
            
            printSentCode();
            
            // Actually send code
            irSender.sendRaw(rawCodes, codeLen, 38);
            
            Display::print("\nWaiting for signal to finish...");
            Display::update();
            
            //Give time to send result without receiving at the same time
            wait(1);
            
        } 
        else if ((mode == Mode::Scan) && (irReceiver.decode(&results))) 
        {
            // Enter Send mode to prevent capturing sent code
            mode = Mode::Send;
            
            Display::clear();
            Display::print("IR Received type = ");
            Display::print(results.decode_type);
            Display::print("\ncode = ");
            Display::print(results.value);
            
            storeCode(results);
            
            // Resume receiver for next time we enter Scan mode
            irReceiver.resume(); 
            
            Display::print("\n\nWaiting for input or command.");
        }
        
        Display::update();
    }
    
    return 0;
}

I definitely intend to clean up the comments and some coding style, so no worries about that part too much.

Third Draft

#include <Pokitto.h>
#include <IRremote/IRremote.h>

// PWM output for IR blaster
IRsend irSender(EXT0);

// Currently only 3+ works for the receiver. 
IRrecv irReceiver(EXT3); 

// Results of reading an incoming code
decode_results results; 

// The durations if raw
unsigned int rawCode[RAWBUF]; 

// The length of the code
std::size_t codeLength; 

// The type denoting the current operation modes
enum class Mode
{
    Send,
    Scan,
};

// The variable containing the currently set opreation mode, defaulting to Send
Mode mode = Mode::Send;

// Stores the code for later sending
void storeCode(decode_results results) 
{
    using Pokitto::Display;
    
    // codeLength is used again when sending the code, so we set it here
    codeLength = results.rawlen - 1;
    
    // To store raw codes:
    // Drop first value (gap)
    // Convert from ticks to microseconds
    for (int i = 1; i <= codeLength; ++i)
    {
        rawCode[i - 1] = results.rawbuf[i] * USECPERTICK;
        
        Display::print(rawCode[i - 1]);
        Display::print(" ");
    }
    Display::print("\n");
}

// Prints the code being sent to the screen
void printSentCode()
{
    using Pokitto::Display;
    
    for (int i = 1; i < results.rawlen; ++i) 
    {
        if ((i % 2) != 0) 
        {
            Display::print(results.rawbuf[i] * USECPERTICK);
        } 
        else
        {
            Display::print('-');
            Display::print((unsigned long) results.rawbuf[i] * USECPERTICK);
        }
        Display::print(" ");
    }
}

int main()
{
    using Pokitto::Core;
    using Pokitto::Display;
    using Pokitto::Buttons;
    
    Core::begin();
    
    Display::persistence = true;
    Display::invisiblecolor = 0;
    Display::setFont(font5x7);
    
    Display::print("PEX IR test\nPress B to scan or A to send.\nC to open this menu.");
    
    // Enable IR In on the receiver so it is ready to scan for IR codes.
    irReceiver.enableIRIn();
    
    while (Core::isRunning())
    {
        if (Buttons::cBtn())
        {
            Display::clear();
            Display::print("Press B to scan or A to send.");
            
            mode = Mode::Send;
        }
        
        if (Buttons::bBtn())
        {
            Display::clear();
            Display::print("Entered scan mode...\n");
            
            mode = Mode::Scan;
        }
        
        if (Buttons::aBtn())
        {
            // Immediately set mode to Send to prevent scanning the sent code
            mode = Mode::Send;
            
            Display::clear();
            Display::print("Sending signal:\n");
            
            printSentCode();
            
            // Send the currently stored code
            irSender.sendRaw(rawCode, codeLength, 38);
        } 
        else if ((mode == Mode::Scan) && (irReceiver.decode(&results))) 
        {
            // Enter Send mode to prevent Scanning the code during Send
            mode = Mode::Send;
            
            Display::clear();
            Display::print("IR Received type = ");
            Display::print(results.decode_type);
            Display::print("\ncode = ");
            Display::print(results.value);
            Display::print("\n");
            
            storeCode(results);
            
            // Resume receiver for next time we enter Scan mode
            irReceiver.resume(); 
            
            Display::print("\n\nWaiting for input or command.");
        }
        
        Display::update();
    }
    
    return 0;
}

I am open to any and all criticism! I’d like this to be as perfect as possible :slight_smile: Thanks in advance!

4 Likes
2

The code is very easy to understand.

A couple of petty things:

codeType could be declared locally in the storeCode() function.

int count = results.rawlen; 
for (int i = 1; i < count; i++) {

could be simplified to: (Unless it is expensive to keep calling the method)

for (int i = 1; i < esults.rawlen; i++) {

Remove the multiple occurences of PD::update(); and put it once at the bottom of the loop.

2 Likes
3

Thanks for the suggestions! I updated the post.

1 Like
4

Think you missed this (important?) one:

Remove the multiple occurences of PD::update(); and put it once at the bottom of the loop.

1 Like
5

I removed one! :laughing:
I had the one before the wait(1) to make sure the text showed up during the pause.

1 Like
6

Oh I see. I always try to use the cycle with a countdown timer rather than wait(). Just a personal preference.

2 Likes
7

Turns out the wait was useless once I incorporated an enum type :slight_smile: thanks for pointing it out.

2 Likes