Reading Pulse Length of a PWM signal (at 1kHz)

Hi,
I’m trying to adapt the firmware of a ATMega328P based board (used for OSD display):

to read on an ADC pin the pulse length of a 1kHz signal coming from a radio control receiver.
I tried using the pulseIn() function but it seems that it is not sufficiently precise for this.
I’ve also tried using another similar function that I saw in another firmware:

uint16_t FastpulseIn(uint8_t pin, uint8_t state, unsigned long timeout)
{
  uint8_t bit = digitalPinToBitMask(pin);
  uint8_t port = digitalPinToPort(pin);
  uint8_t stateMask = (state ? bit : 0);
  uint16_t width = 0;
  unsigned long numloops = 0;
  unsigned long maxloops = timeout;
  
  while ((*portInputRegister(port) & bit) == stateMask)
    if (numloops++ == maxloops)
      return 0;
  
  while ((*portInputRegister(port) & bit) != stateMask)
    if (numloops++ == maxloops)
      return 0;
  
  while ((*portInputRegister(port) & bit) == stateMask) {
    if (numloops++ == maxloops)
      return 0;
    width++;
  }
  return width; 
}

which gave better results but not with the precision desired.
The file is this:
https://code.google.com/p/minoposd/source/browse/trunk/minOPOSD_additionals_only/AnalogRssi.ino

I’m trying to replace the lines:

osd_rssi = analogRead(RSSI_PIN) / 4;

To have the pulse length coming from that pin.
Is there a better way to do this?
Thanks in advance

helioteixeira:
I'm trying to replace the lines:

osd_rssi = analogRead(RSSI_PIN) / 4;

To have the pulse length coming from that pin.

I don't understand.

Do you mean that you want the analogRead() to read from a different pin?
Or do you mean something else entirely.

It is usually a good idea to post a complete program, not just a snippet.

...R

Connect your PWM signal to a digital pin (4 for example) and run this simple sketch just for curiosity.

uint32_t dur;

void setup() {
  Serial.begin(9600);
  pinMode(4,INPUT);
  
}

void loop() {
  while(PIND & bit(4));
  dur = micros();
  while(!(PIND & bit(4)));
  dur = micros() - dur;
  Serial.println(dur);
  delay(200); 
}

jcallen:
run this simple sketch just for curiosity.

Whose curiosity ?

It is a good idea to explain to the OP why a particular test should be tried and what output is expected.

…R

Robin2:
I don't understand.

Do you mean that you want the analogRead() to read from a different pin?
Or do you mean something else entirely.

It is usually a good idea to post a complete program, not just a snippet.

...R

I included a link to the complete file in the original post:
https://code.google.com/p/minoposd/source/browse/trunk/minOPOSD_additionals_only/AnalogRssi.ino

helioteixeira:
I included a link to the complete file in the original post:
https://code.google.com/p/minoposd/source/browse/trunk/minOPOSD_additionals_only/AnalogRssi.ino

I confess I had not seen the link. However I am not going to another website to look for your code. Post it here. Add the .ino file as an attachment if it is too long.

You did not answer my question, even though you quoted it.

…R

jcallen:
Connect your PWM signal to a digital pin (4 for example) and run this simple sketch just for curiosity.

uint32_t dur;

void setup() {
  Serial.begin(9600);
  pinMode(4,INPUT);
 
}

void loop() {
  while(PIND & bit(4));
  dur = micros();
  while(!(PIND & bit(4)));
  dur = micros() - dur;
  Serial.println(dur);
  delay(200);
}

I'm sorry for the late answer. The thing is that this "modded" board doesn't expose any digital pin :confused:
I only have access to pins 23(ADC0),24(ADC1),25(ADC2),26(ADC3) and 19(ADC6).
I've been trying whitout success to weld a wire to the pin, but its too small for my hands...

helioteixeira:
I've been trying whitout success to weld a wire to the pin, but its too small for my hands...

I hope you mean "solder"

In any case, you have still not responded to requests to post your own code so we can view it easily. The ball is in your court.

...R

Here it is:

Analog.h:

/**
 ******************************************************************************
 *
 * @file       AnalogRssi.h
 * @author     Philippe Vanhaesnedonck
 * @brief      Implements RSSI report on the Ardupilot Mega MinimOSD
 * 	       using built-in ADC reference.
 * @see        The GNU Public License (GPL) Version 3
 *
 *****************************************************************************/
/*
 * This program is free software; you can redistribute it and/or modify
 * it under the terms of the GNU General Public License as published by
 * the Free Software Foundation; either version 3 of the License, or
 * (at your option) any later version.
 *
 * This program is distributed in the hope that it will be useful, but
 * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
 * or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
 * for more details.
 *
 * You should have received a copy of the GNU General Public License along
 * with this program; if not, see <http://www.gnu.org/licenses/> or write to the 
 * Free Software Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
 */


// !!! For using this, you have to solder a little bit on the MinimOSD, see the wiki !!!


#ifndef ANALOG_RSSI_H_
#define ANALOG_RSSI_H_

//#define RSSI_PIN			1			// A1 is pin 24
#define RSSI_PIN      3     // ADC3 is pin 26
#define PWMRSSIPIN    A3 
								// Any 'free' analog input will do -- A0-5 are available, 
								// which are pins 23-28 on the ATmega328p

#define REF_VOLTAGE			1.1			// INTERNAL: a built-in reference, equal to 1.1 volts on the ATmega168 or ATmega328

void analog_rssi_init(void);
void analog_rssi_read(void);


#endif /* ANALOG_RSSI_H_ */

Analog.ino:

/**
 ******************************************************************************
 *
 * @file       AnalogRssi.ino
 * @author     Philippe Vanhaesnedonck
 * @brief      Implements RSSI report on the Ardupilot Mega MinimOSD
 * 	       using built-in ADC reference.
 * @see        The GNU Public License (GPL) Version 3
 *
 *****************************************************************************/
/*
 * This program is free software; you can redistribute it and/or modify
 * it under the terms of the GNU General Public License as published by
 * the Free Software Foundation; either version 3 of the License, or
 * (at your option) any later version.
 *
 * This program is distributed in the hope that it will be useful, but
 * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
 * or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
 * for more details.
 *
 * You should have received a copy of the GNU General Public License along
 * with this program; if not, see <http://www.gnu.org/licenses/> or write to the 
 * Free Software Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
 */


// !!! For using this, you have to solder a little bit on the MinimOSD, see the wiki !!!


#include "AnalogRssi.h"


void analog_rssi_init(void)
{
	analogReference(INTERNAL);			// INTERNAL: a built-in reference, equal to 1.1 volts on the ATmega168 or ATmega328
}


void analog_rssi_read(void)
{
	if (rssiraw_on) {
		//osd_rssi = analogRead(RSSI_PIN) / 4;          // Just raw value, 0-255. We use this range to better align
		//osd_rssi = (uint8_t)(float)((osd_chan7_raw - 992) * .31135f); 

    // This only works for Frsky X8R Rx
    // Currently this receiver has a RSSI port which outputs PWM signal of 1kHz whose duty cycle (%) is equivalent to RSSI (%)
    uint16_t rssipulselength;
    rssipulselength = (uint16_t)FastpulseIn(PWMRSSIPIN, HIGH,2048); //Returns the pulse length in microseconds
    //rssipulselength = PulseIn(PWMRSSIPIN, HIGH,1024); //Returns the pulse length in microseconds -> Not very precise
    
    //For a frequency of 1kHz the cycle duration is 1000us (1ms). 
    uint16_t dutycycle;
    
    dutycycle = (uint16_t)(float)(rssipulselength * .1f); // DutyCycle = PulseLength / CycleDuration * 100

    if (dutycycle==0) {
      osd_rssi=osd_rssi; //Keeps the last value
    }
    else if (dutycycle>254) {
      osd_rssi=255;
    } else {
      osd_rssi = (uint8_t)(dutycycle);
    }
    
	} else {
#ifdef JR_SPECIALS
// SEARCH GLITCH
		osd_rssi = analogRead(RSSI_PIN)       / 4;			// 1:1 input
#else
		osd_rssi = analogRead(RSSI_PIN) * .2  / 4 + osd_rssi * .8;	// Smooth input
    //osd_rssi = (uint8_t)(float)(((osd_chan7_raw - 992) * .31135f) * .2 + osd_rssi * .8);
#endif
		osd_rssi = constrain(osd_rssi, rssipersent, rssical);		// Ensure we stay in range
	}
}

uint16_t FastpulseIn(uint8_t pin, uint8_t state, unsigned long timeout)
{
  uint8_t bit = digitalPinToBitMask(pin);
  uint8_t port = digitalPinToPort(pin);
  uint8_t stateMask = (state ? bit : 0);
  uint16_t width = 0;
  unsigned long numloops = 0;
  unsigned long maxloops = timeout;
  
  while ((*portInputRegister(port) & bit) == stateMask)
    if (numloops++ == maxloops)
      return 0;
  
  while ((*portInputRegister(port) & bit) != stateMask)
    if (numloops++ == maxloops)
      return 0;
  
  while ((*portInputRegister(port) & bit) == stateMask) {
    if (numloops++ == maxloops)
      return 0;
    width++;
  }
  return width; 
}

In attachment is the main minOPOSD.ino

minOPOSD.ino (8.56 KB)

Can you explain what each of the three program files is for - especially the two .ino files.

Is it the case that the total project consists of the code in the 3 files ?

...R

The minOPOSD.ino reads comunicates with an flight controller using a protocol called UAVTalk.
It gets all the info for the FC and also exposes pins for connecting battery, current sensor, RSSI, videoIn and videoOut and overlays in the video all the OSD info.
In attachment all the project files

AnalogRssi.h (1.69 KB)

AnalogRssi.ino (3.42 KB)

ArduCam_Max7456.cpp (7.48 KB)

ArduCam_Max7456.h (2.83 KB)

ArduNOTES.ino (2.55 KB)

BOOT_Func.ino (2.09 KB)

FlightBatt.h (3.02 KB)

FlightBatt.ino (2.46 KB)

Font.ino (2.54 KB)

MAVLink.ino (7.08 KB)

minOPOSD.ino (8.56 KB)

OSD_Config.h (7.42 KB)

OSD_Config_Func.ino (16.3 KB)

OSD_Func.h (5.53 KB)

OSD_Panels.ino (48.6 KB)

OSD_Vars.h (5.51 KB)

PacketRxOk.h (1.63 KB)

PacketRxOk.ino (2.41 KB)

Spi.cpp (1.19 KB)

Spi.h (461 Bytes)

Sorry. Too many files. I am very lazy.

Can you write a short program (say 20 to 100 lines) that demonstrates the problem ?

…R

This is it:

uint16_t FastpulseIn(uint8_t pin, uint8_t state, unsigned long timeout)
{
  uint8_t bit = digitalPinToBitMask(pin);
  uint8_t port = digitalPinToPort(pin);
  uint8_t stateMask = (state ? bit : 0);
  uint16_t width = 0;
  unsigned long numloops = 0;
  unsigned long maxloops = timeout;
  
  while ((*portInputRegister(port) & bit) == stateMask)
    if (numloops++ == maxloops)
      return 0;
  
  while ((*portInputRegister(port) & bit) != stateMask)
    if (numloops++ == maxloops)
      return 0;
  
  while ((*portInputRegister(port) & bit) == stateMask) {
    if (numloops++ == maxloops)
      return 0;
    width++;
  }
  return width; 
}

This function mimics pulseIn() although with a more proximate value, but still not enough. Is there an even better alternative?

helioteixeira:
This is it:

Sorry. Not what I had in mind. I said program, not snippet of a program.

Without seeing the code in the context of a complete working program (or at least a compilable failing program) I always worry that there is a piece I can't see that may be making nonsense of my thought process. Actually the truth is that my thought process just does not work - my mind goes round in cicrcles.

You, on the other hand, are well aware of the context in which your snippet exists.

...R

Here it is:

uint16_t dur;

void setup() {
  Serial.begin(9600);
  pinMode(A3, INPUT); //ADC3 is pin 26
  
}

void loop() {
  dur = FastpulseIn(A3, HIGH,1024);
  Serial.println(dur);
  delay(200); 
}

uint16_t FastpulseIn(uint8_t pin, uint8_t state, unsigned long timeout)
{
  uint8_t bit = digitalPinToBitMask(pin);
  uint8_t port = digitalPinToPort(pin);
  uint8_t stateMask = (state ? bit : 0);
  uint16_t width = 0;
  unsigned long numloops = 0;
  unsigned long maxloops = timeout;
  
  while ((*portInputRegister(port) & bit) == stateMask)
    if (numloops++ == maxloops)
      return 0;
  
  while ((*portInputRegister(port) & bit) != stateMask)
    if (numloops++ == maxloops)
      return 0;
  
  while ((*portInputRegister(port) & bit) == stateMask) {
    if (numloops++ == maxloops)
      return 0;
    width++;
  }
  return width; 
}

helioteixeira:
Here it is:

An explanation would be helpful. It takes a long time to figure out what code does just by reading it.

It has taken a long time to get here. Please remind me what the problem is and how that code demonstrates it.

what does *portInputRegister(port) do, and where is it defined ?

...R

Hi,

Thank you for your patience in helping solving this problem.

The explanation:

uint16_t dur; // declares a variable which will hold the pulse length

void setup() {
  Serial.begin(9600); // starts serial communication
  pinMode(A3, INPUT); //Configures pin 26 (ADC) for input
  
}

void loop() {
  dur = FastpulseIn(A3, HIGH,1024); //Calls FastpulseIn
  Serial.println(dur); //Prints the pulse duration to serial port
  delay(200); //Pauses the programm 200miliseconds
}

uint16_t FastpulseIn(uint8_t pin, uint8_t state, unsigned long timeout)
{
  // cache the port and bit of the pin in order to speed up the
  // pulse width measuring loop and achieve finer resolution.
  uint8_t bit = digitalPinToBitMask(pin);
  uint8_t port = digitalPinToPort(pin);
  uint8_t stateMask = (state ? bit : 0);
  uint16_t width = 0;
  unsigned long numloops = 0;
  unsigned long maxloops = timeout;
  
  // wait for any previous pulse to end
  while ((*portInputRegister(port) & bit) == stateMask)
    if (numloops++ == maxloops)
      return 0; //Return 0 if timed-out
  
  // wait for the pulse to start
  while ((*portInputRegister(port) & bit) != stateMask)
    if (numloops++ == maxloops)
      return 0; //Return 0 if timed-out
  
  // wait for the pulse to stop
  while ((*portInputRegister(port) & bit) == stateMask) {
    if (numloops++ == maxloops)
      return 0; //Return 0 if timed-out
    width++;
  }
  return width; 
}

The problem is that neither pulseIn() or FastpulseIn() returns the real pulse width (measured with an oscilloscope). Is there a more precise function for this?

You surely know that portInputRegister() is an Arduino core macro that:
“…returns an input port register of the specified port”
and is defined in Arduino.h core library…

Have you considered doing this

 // wait for the pulse to stop
  startMicros = micros();
  while ((*portInputRegister(port) & bit) == stateMask) {
    if (numloops++ == maxloops)
      return 0; //Return 0 if timed-out
  }
  endMicros = micros();
  width = endMicros - startMicros;

I don't think I would use those Arduino macros unless I was satisfied that they are very fast. I would just use PINx where x is the appropriate port, and mask the value myself - just takes two machine cycles

dataRead = PINB; // assuming you want to read PortB
dataRead &= bitMask;

I don't see any need to create general purpose code for a specific and unusual application.

...R