Forum Rule: Always post complete source code & details to reproduce any issue!
Results 1 to 19 of 19

Thread: Teensy 3.2, part of my code will only read pins as HIGH and I'm really puzzled

  1. #1
    Junior Member
    Join Date
    Mar 2021
    Posts
    8

    Teensy 3.2, part of my code will only read pins as HIGH and I'm really puzzled

    I'm a retired EE and decided I'd tackle one more project. this time something that has no deadline or budgetary constraints because is just something I'd like to tryout. First let me say that I'm new to the Teensy world and I'm not completely fluent in Cpp but I do have abt 50 yrs of component level design under my belt and hundreds of thousands of lines of code behind me on microchip, phillips etc. but that was almost entirely in assembly language.

    Here's my problem: the attached code runs perfectly except for the section "heattimer". Here the digitalReadFast returns a "true" under all conditions. I've switched pins; tried different definitions etc.

    I'm hoping that someone here can put me on the right track.

    Roger
    Attached Files Attached Files

  2. #2
    Senior Member+ defragster's Avatar
    Join Date
    Feb 2015
    Posts
    15,274
    This looks to be the problem - always set TRUE - the correct one should set =false:
    Code:
    void RHeaterISR() {                 // Run Heater Button Input Interrup Service Routine
      delay (100);                       // Short debounce on RUN_PIN switch input
      if (digitalReadFast (RUN_PIN)){   // RUN Heater routine starts when switch is released
      rhFlag = true;                    // Set rhFlag 
    
     } else {
    
      rhFlag = true;                    // RUN_PIN switch has been released so set rhFlag to true               
     }
     }

  3. #3
    Senior Member
    Join Date
    Apr 2014
    Location
    Germany
    Posts
    9,435
    or just rhFlag = digitalReadFast(..)

    delay() in a isr not a good idea, too...
    Then, rhFlag should be declared to be volatile.:

    volatile bool rhFlag = false;


    edit: Or just use the bounce library. 100ms delay does not look like you need interrupts It looks like debouncing?

  4. #4
    Senior Member+ defragster's Avatar
    Join Date
    Feb 2015
    Posts
    15,274
    Quote Originally Posted by Frank B View Post
    or just rhFlag = digitalReadFast(..)

    delay() in a isr not a good idea, too...
    Then, rhFlag should be declared to be volatile.:

    volatile bool rhFlag = false;
    All True. ....

  5. #5
    Junior Member
    Join Date
    Mar 2021
    Posts
    8
    Quote Originally Posted by Frank B View Post
    or just rhFlag = digitalReadFast(..)

    delay() in a isr not a good idea, too...
    Then, rhFlag should be declared to be volatile.
    I understand abt the delay in a ISR. Iíll take care of that. The failure Iím having is when the heattimer routine is called. Are you saying that I shouldnít or canít use the digitalReadFast inside an ďifĒ statement?

  6. #6
    Senior Member+ defragster's Avatar
    Join Date
    Feb 2015
    Posts
    15,274
    delay() is always best avoided ... especially in an _ISR()

    glancing a bit ... if the heater were on for a fixed time - and the button changed - it could add 100 ms per change preventing the accurate timing of the heater on time.

    Look on pjrc.com for elapsedMillis - and sample usage in examples. It keeps the code flowing where delay() brings it to a halt preventing timely processing and function.

    Teensy is very fast - T_3.2 might enjoy many 10's of thousands of iterations passing through loop() when there are no delay()'s in the code to running the loop() and polling things can be quite accurate.

  7. #7
    Senior Member+ defragster's Avatar
    Join Date
    Feb 2015
    Posts
    15,274
    Quote Originally Posted by rlmnky View Post
    I understand abt the delay in a ISR. I’ll take care of that. The failure I’m having is when the heattimer routine is called. Are you saying that I shouldn’t or can’t use the digitalReadFast inside an “if” statement?
    See post #2 for the source of current problem not seeing the pin go false.

    An if() is fine. But any variable in an _ISR() should be declared as volatile to assure the code isn't using a copy of the variable that the _ISR() could change without proper reference.

  8. #8
    Senior Member
    Join Date
    Apr 2014
    Location
    Germany
    Posts
    9,435
    Yup, the main problem is you don't set it to false, as defragster says.

    you can use the "if" - no problem.
    rhFlag = digitalRead is just less to write and a bug like setting it to true in both cases can not happen.

    But the volatile is important, too... you must tell the compiler not to optimize that away.

  9. #9
    Junior Member
    Join Date
    Mar 2021
    Posts
    8
    Thanks to both of you. I’ll go back and change those things and let you know how it goes.

  10. #10
    Junior Member
    Join Date
    Mar 2021
    Posts
    8
    And yes, that 100ms inside the ISR was just for denounce and I can see that would be bad practice in most code since it will defeat the main usefulness of an ISR - fast response to a a randomly occurring event.

  11. #11
    Senior Member+ defragster's Avatar
    Join Date
    Feb 2015
    Posts
    15,274
    Bouncy bits in an ISR are trouble - especially marked CHANGE - no state can be assumed and it will want to change twice for every return to the new state.

    There is a BOUNCE and BOUNCE2 library that can monitor the pins in a tested way to return known debounced state - then local _ISR() and added interrupt could be removed and the library will give a debounced value of the monitored pin.

    Welcome to Teensy - there are examples for Bounce/Bounce2 and BlinkWithoutDelay and others installed with TeensyDuino that will show elapsedMillis or elapsedMicros in use as well as the forum for existing posts that are quite helpful. Many things on PJRC.com are findable - though some may mislead as written for 8 bit AVR's to read with care and divert to installed examples for details.

    And no need to worry about cpp - the toolchain is just as happy with "C".

  12. #12
    Senior Member
    Join Date
    Apr 2014
    Location
    Germany
    Posts
    9,435
    Take a look here: https://www.pjrc.com/teensy/td_libs_Bounce.html

    Yes, I wrote a lot of assembler, too... and parts of your code look like assembler
    That's not bad at all, but you could just use the "proven to work" tools of others and use that library.
    100 milliseconds is an incredible long time for any Teensy.

    You'll can get in trouble interrupts with the same priority, too, if you wait 100 milliseconds. Not sure which priority i.e. the USB has..

  13. #13
    Junior Member
    Join Date
    Mar 2021
    Posts
    8
    Iím already planning to eliminate the ISRs.

    Your comment abt my code looking like .asm code made me smile. Old habits are hard to break.

  14. #14
    Senior Member
    Join Date
    Apr 2014
    Location
    Germany
    Posts
    9,435
    If you have some time or are interested, you could take a look at the ARM Cortex assembler... there are some *amazing* things, especially with the "DSP" part.
    What can be done in one machine cycle is really amazing. Then, then Teensy 4 CPU can do some of these with "dual issue".. (not to mention the branch prediction or speculative loads)

  15. #15
    Junior Member
    Join Date
    Mar 2021
    Posts
    8
    okay, i took everything out of the rheatISR and added "rhFlag = digitalReadFast (RUN_PIN);" as suggested. That runs and has the added benefit of allowing the operator to keep the heater active for as long as he switch is closed. but... the digitalReadFasts in the "heattimer" routine still return as true (HIGH) for the four input pins being read even though they are tied to ground during this test. as you can see I'm using the USB

    BTW, Since this code is all working pretty much as desired, I'm playing with the bounce2 routine in the library on the side so I dont mess this code up. I've got it running but this same problem exists there too.

  16. #16
    Senior Member+ defragster's Avatar
    Join Date
    Feb 2015
    Posts
    15,274
    Those 'HexBit0' things are just pins - with INPUT_PULLUP that will pull them to 1 if not truly grounded.

    If they are always 1 then it seems they are seeing high voltage when measured. Given a working Teensy and proper wiring - the problem must be in the code ...

    The problem is in the code. All four IF() statements have this problem:
    Code:
          if (digitalReadFast(HexBit0));{  // Read the switch bit by bit
            htime = (htime + 1);
    //                Serial.print(htime, DEC);   // print function used for testing only
           }
    Writing that out with better indentation they are executed as follows:
    Code:
      if (digitalReadFast(HexBit0));
    
      { // Read the switch bit by bit
        htime = (htime + 1);
    //                Serial.print(htime, DEC);   // print function used for testing only
      }
    The if() executes a single statement - as above that single statement is ";"

    Those spare semi colons need to be removed.

    Since the Arduino IDE is in use - commanding with "Ctrl + T" will perform a unified formatting of the code. It can often result in adding white space in a way that can point out easy to overlook errors.

    > In this case it just adds a 'space' after that evil ; [ at least in my editor - not the IDE ]

  17. #17
    Junior Member
    Join Date
    Mar 2021
    Posts
    8
    I believe i grasped that and indeed that fixed the issue. it reads the switch perfectly now. Here'd the revised routine. thanks gentlemen. I"ll sleep better tonight.

    Code:
    void heattimer(){                      // Delay for selected time (Hexidecimal value read from 16-
                                           // position rotary switch) while Induction Heater is on
    
           htime = 0;                      // Start with htime variable at a known value
    
          if (digitalReadFast(HexBit0)){  // Read the switch bit by bit
            htime = (htime + 1);
    //                Serial.print(htime, DEC);   // print function used for testing only
          }
                        
          if (digitalReadFast(HexBit1)){
            htime = (htime + 2);
    //               Serial.print(htime, DEC);
                         
          }
            
          if (digitalReadFast(HexBit2)){
           htime = (htime + 4);
    //               Serial.print(htime, DEC);
          }
          
         if (digitalReadFast(HexBit3)){
          htime = (htime + 8);    
          }
    
          Serial.print("htime: Decimal = "); // Print function used for testing only
          Serial.print(htime, DEC);
          Serial.print("; Hexidecimal = ");
          Serial.print(htime, HEX);
          Serial.print("; Binary = ");
          Serial.println(htime, BIN);
          
          delay (500 * htime);            // Delay 500ms * htime value read from 16 position switch
        }
    Last edited by defragster; 03-10-2021 at 12:13 AM. Reason: Put CODE block around selected code - with # on toolbar

  18. #18
    Senior Member+ defragster's Avatar
    Join Date
    Feb 2015
    Posts
    15,274
    Odd concept ... too many semi-colons ... I still leave them off and have to recompile

    That looks much better and right to work. As noted "C" conditional and control elements are designed to execute any single statement that can be " { } " or " ; "

    Remember the Ctrl+T in the IDE - it can help learning or knowing C to visualize how the compiler might see the code.

    If you have a better favorite editor the preferences can be set in the IDE to 'use external editor [ Ctrl + , ]. IDE still does building and upload - but files there are Read Only and updated before building.

    Using SublimeText here and created a set of batch files (extended from Frank B initial work ) that trigger a build from command line within the editor ( tested it the other week to work on Notepad++ as well ) :: github.com/Defragster/Tset

    Being on Windows that can work there. It uses the Full normal install and build features of the IDE and TeensyDuino - but also the coolest non-PJRC thing about Teensy ... TyCommander can be integrated to handle the uploads ( or not ) and work as a better Serial Monitor. Make using one ( or more ) Teensy much more fun and productive.

  19. #19
    Junior Member
    Join Date
    Mar 2021
    Posts
    8
    per your suggestions, i removed the ISRs and incorporated the bounce2 routines. this works perfectly. my sincere thanks to you both.

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •