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

rlmnky

Member
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
 

Attachments

  • resonanceFinder.ino
    9.1 KB · Views: 59
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
 [B] rhFlag = true;                    // Set rhFlag [/B]

 } else {

  [B][COLOR="#FF0000"]rhFlag = true;                    // RUN_PIN switch has been released so set rhFlag to true   [/COLOR][/B]            
 }
 }
 
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?
 
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?
 
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.
 
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.
 
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.
 
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.
 
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".
 
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..
 
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.
 
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)
 
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.
 
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))[B][COLOR="#FF0000"];[/COLOR][/B]{  // 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))[B][COLOR="#FF0000"];[/COLOR][/B]

  { // 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 ]
 
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 a moderator:
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.
 
per your suggestions, i removed the ISRs and incorporated the bounce2 routines. this works perfectly. my sincere thanks to you both.
 
Back
Top