Hello Friends :)

I’ve been re-teaching myself Arduino to brush up on my C/C++ and I’ve been working on this project for a simple adding machine with two user inputs and a single visual output. I’m not sure what is wrong with my code.

As it stands, the code compiles without error or warning and runs on my Uno R3, but as soon as I press “NumberEntryButton” to begin building the first number, pin 13 immediately flashes 8 times, then stays solid on.

If anyone has advice or help to offer, I’d super appreciate it.

I’ve attached a permalink to the file in my git repo.

  • one_old_coder@piefed.social
    link
    fedilink
    English
    arrow-up
    4
    ·
    edit-2
    2 days ago

    My 2 shitty cents because I haven’t done Arduino for a long time:

    • AddTwoNumbers and ShowResultToUser are useless. Put their content in the loop() function because it’s very unlikely you will reuse such a code.
    • The for loop in ShowResultToUser is strange. You’re calling AddTwoNumbers all the time. Maybe do something like for (int i = 0, n = GetNumber() + GetNumber(); i < n; i++)? That’s more traditional.
    • In FlashLed, I would add another delay at the end, like show/delay/hide/delay or something.
    • The GetNumber function is confusing, add a comment to explain what it does.
    • Move all the constants as global constants. You have 8 and 9 in setup and GetNumber, 13 in setup and FlashLed, but what happens when you want to change a pin? You have to change the values everywhere.
  • Alphenex53@programming.dev
    link
    fedilink
    English
    arrow-up
    2
    ·
    2 days ago

    You can try this I kinda half-arsed it though. I personally could not see the issue in your code other than the fact that you are writing bad/ugly code. I don’t mean it in a rude way, you should read other people’s code and understand the conventions. Besides that, I think the issue is with the delays.

    // This is a lot more understandable now
    #define ENTRY_FINISH_PIN 8
    #define ENTRY_NUMBER_PIN 9
    #define ENTRY_REPORT_PIN 13
    
    #define ENTRY_DELAY_MS 500
    #define ENTRY_REPORT_SHOW_MS 500
    #define ENTRY_REPORT_WAIT_MS 100
    
    int GetNumber()
    {
        // loop until ENTRY_FINISH_PIN is HIGH
        // if ENTRY_NUMBER_PIN is HIGH, add to the iterant and turn on the LED
        // after that wait a bit and turn off LED
        
        int iterant = 0;
        while (digitalRead(ENTRY_FINISH_PIN) != HIGH) {
            if (digitalRead(ENTRY_NUMBER_PIN == HIGH)) {
                iterant++;
    
                digitalWrite(ENTRY_REPORT_PIN, HIGH);
            }
    
            delay(ENTRY_DELAY_MS);
            digitalWrite(ENTRY_REPORT_PIN, LOW);
        }
        
        return iterant;
    }
    
    void setup()
    {
        pinMode(ENTRY_FINISH_PIN, INPUT);
        pinMode(ENTRY_NUMBER_PIN, INPUT);
        pinMode(ENTRY_REPORT_PIN, OUTPUT);
    }
    
    void loop()
    {
        int sum = GetNumber() + GetNumber();
    
        for (int i = 0; i < sum; i++) {
            digitalWrite(ENTRY_REPORT_PIN, HIGH);
            delay(ENTRY_REPORT_SHOW_MS);
            digitalWrite(ENTRY_REPORT_PIN, LOW);
            delay(ENTRY_REPORT_WAIT_MS); // waiting a bit more makes sense imo
        }
    }
    

    I wish u luck with your Arduino adventure ^^

  • okwhateverdude@lemmy.world
    link
    fedilink
    English
    arrow-up
    2
    ·
    2 days ago

    So the first thing I can recommend for you to try is to write out, line by line, in plain text what you expect the program to do at that point in the program and what the current state is of the program at that point.

    You’re making some code idioms that are more complex than they should be which is likely leading to the mismatch in your expectation vs. actual program execution. Simplify those bits, use more temporary variables. Very likely your read loops are not doing what you think they should be doing.