Adafruit Industries, Essential service and business: NYC – Executive Order 202.6 - Read more. Accepting essential orders - here's how.
0

AFSoftSerial improvement
Moderators: adafruit_support_bill, adafruit

Please be positive and constructive with your questions and comments.

AFSoftSerial improvement

by mikalhart on Sat Jan 03, 2009 7:39 pm

@ladyada--

I was playing with AFSoftSerial over the holiday -- thanks for making that, by the way: it's a tremendously valuable addition to the Arduino repertory -- and I discovered a small flaw. When communicating with my EM-604A GPS unit, I noticed that I would occasionally drop an incoming character. The drop always corresponded with a time during which I was doing more than the usual amount of processing in between calls to AFSoftSerial::read(), but, and I can't overstate this, it was not due to a buffer overflow. (Careful experiments showed that no more than 15 of 64 bytes in the buffer were ever being consumed at a given time by my program.)

The problem occurs when the buffer starts getting a little bit full, say 10-15 bytes, so this little piece of code from read() starts getting longer:

Code: Select all | TOGGLE FULL SIZE
  // if we were awesome we would do some nifty queue action
  // sadly, i dont care
  for (i=0; i<_receive_buffer_index; i++) {
    _receive_buffer[i] = _receive_buffer[i+1];
  }
  _receive_buffer_index--;


What I believe is happening is that as this loop gets longer (i.e. as _receive_buffer_index gets big), the probability that a new character will arrive during the loop gets proportionally larger. But because accesses within the loop to _receive_buffer_index are optimized away, the newly arrived character is effectively discarded. Is that clear?

Consider if _receive_buffer_index is 15. At the beginning of the loop, 15 is loaded into a register and the loop is executed 15 times. Then 14 is stored in _receive_buffer_index, even if a character arrives during the loop execution.

The easy solution for this of course is to simply declare _receive_buffer_index volatile. This seemed to solve the problem for me.

But even better is to turn the buffer index into "head" and "tail" pointers to implement that circular buffer that you "don't care" about. :) If you are interested, I offer the revision below which does this, I think successfully. (I'm using it in my GPS project.)

Code: Select all | TOGGLE FULL SIZE
/*
  SoftwareSerial.cpp - Software serial library
  Copyright (c) 2006 David A. Mellis.  All right reserved. - hacked by ladyada

  This library is free software; you can redistribute it and/or
  modify it under the terms of the GNU Lesser General Public
  License as published by the Free Software Foundation; either
  version 2.1 of the License, or (at your option) any later version.

  This library 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
  Lesser General Public License for more details.

  You should have received a copy of the GNU Lesser General Public
  License along with this library; if not, write to the Free Software
  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
*/

/******************************************************************************
 * Includes
 ******************************************************************************/
#include <avr/interrupt.h>
#include "WConstants.h"
#include "AFSoftSerial.h"

/******************************************************************************
 * Definitions
 ******************************************************************************/

#define AFSS_MAX_RX_BUFF 64

/******************************************************************************
 * Statics
 ******************************************************************************/
static uint8_t _receivePin;
static uint8_t _transmitPin;
static int _bitDelay;

static char _receive_buffer[AFSS_MAX_RX_BUFF];
static volatile uint8_t _receive_buffer_tail;
static volatile uint8_t _receive_buffer_head;

#if (F_CPU == 16000000)
void whackDelay(uint16_t delay) {
  uint8_t tmp=0;

  asm volatile("sbiw    %0, 0x01 \n\t"
          "ldi %1, 0xFF \n\t"
          "cpi %A0, 0xFF \n\t"
          "cpc %B0, %1 \n\t"
          "brne .-10 \n\t"
          : "+r" (delay), "+a" (tmp)
          : "0" (delay)
          );
}
#endif

/******************************************************************************
 * Interrupts
 ******************************************************************************/

SIGNAL(SIG_PIN_CHANGE0)
{
  if ((_receivePin >= 8) && (_receivePin <= 15))
  {
    recv();
  }
}

SIGNAL(SIG_PIN_CHANGE2)
{
  if (_receivePin < 8)
  {
    recv();
  }
}


void recv(void) {
  char i, d = 0;
  if (digitalRead(_receivePin))
    return;       // not ready!
  whackDelay(_bitDelay - 8);
  for (i=0; i<8; i++) {
    //PORTB |= _BV(5);
    whackDelay(_bitDelay*2 - 6);  // digitalread takes some time
    //PORTB &= ~_BV(5);
    if (digitalRead(_receivePin))
      d |= (1 << i);
   }
  whackDelay(_bitDelay*2);

  // buffer full?
  if ((_receive_buffer_tail + 1) % AFSS_MAX_RX_BUFF == _receive_buffer_head)
    return;
  _receive_buffer[_receive_buffer_tail++] = d; // save data
  if (_receive_buffer_tail == AFSS_MAX_RX_BUFF)
     _receive_buffer_tail = 0;
}
 


/******************************************************************************
 * Constructors
 ******************************************************************************/

AFSoftSerial::AFSoftSerial(uint8_t receivePin, uint8_t transmitPin)
{
  _receivePin = receivePin;
  _transmitPin = transmitPin;
  _baudRate = 0;
}

void AFSoftSerial::setTX(uint8_t tx) {
  _transmitPin = tx;
}
void AFSoftSerial::setRX(uint8_t rx) {
  _receivePin = rx;
}

/******************************************************************************
 * User API
 ******************************************************************************/

void AFSoftSerial::begin(long speed)
{
  pinMode(_transmitPin, OUTPUT);
  digitalWrite(_transmitPin, HIGH);

  pinMode(_receivePin, INPUT);
  digitalWrite(_receivePin, HIGH);  // pullup!

  _baudRate = speed;
  switch (_baudRate) {
  case 115200: // For xmit -only-!
    _bitDelay = 4; break;
  case 57600:
    _bitDelay = 14; break;
  case 38400:
    _bitDelay = 24; break;
  case 31250:
    _bitDelay = 31; break;
  case 19200:
    _bitDelay = 54; break;
  case 9600:
    _bitDelay = 113; break;
  case 4800:
    _bitDelay = 232; break;
  case 2400:
    _bitDelay = 470; break;
  default:
    _bitDelay = 0;
  }   

   if (_receivePin < 8) {
     // a PIND pin, PCINT16-23
     PCMSK2 |= _BV(_receivePin);
     PCICR |= _BV(2);
  } else if (_receivePin <= 15) {
    // a PINB pin, PCINT0-7
    PCICR |= _BV(0);   
    PCMSK0 |= _BV(_receivePin-8);
  }

  whackDelay(_bitDelay*2); // if we were low this establishes the end
}

int AFSoftSerial::read(void)
{
  uint8_t d;

  if (_receive_buffer_head == _receive_buffer_tail)
    return -1;

  d = _receive_buffer[_receive_buffer_head++]; // grab first byte
  if (_receive_buffer_head == AFSS_MAX_RX_BUFF)
     _receive_buffer_head = 0;
  return d;
}

uint8_t AFSoftSerial::available(void)
{
  return (_receive_buffer_tail + AFSS_MAX_RX_BUFF - _receive_buffer_head) % AFSS_MAX_RX_BUFF;
}

void AFSoftSerial::print(uint8_t b)
{
  if (_baudRate == 0)
    return;
  byte mask;

  cli();  // turn off interrupts for a clean txmit

  digitalWrite(_transmitPin, LOW);  // startbit
  whackDelay(_bitDelay*2);

  for (mask = 0x01; mask; mask <<= 1) {
    if (b & mask){ // choose bit
      digitalWrite(_transmitPin,HIGH); // send 1
    }
    else{
      digitalWrite(_transmitPin,LOW); // send 1
    }
    whackDelay(_bitDelay*2);
  }
 
  digitalWrite(_transmitPin, HIGH);
  sei();  // turn interrupts back on. hooray!
  whackDelay(_bitDelay*2);
}

void AFSoftSerial::print(const char *s)
{
  while (*s)
    print(*s++);
}

void AFSoftSerial::print(char c)
{
  print((uint8_t) c);
}

void AFSoftSerial::print(int n)
{
  print((long) n);
}

void AFSoftSerial::print(unsigned int n)
{
  print((unsigned long) n);
}

void AFSoftSerial::print(long n)
{
  if (n < 0) {
    print('-');
    n = -n;
  }
  printNumber(n, 10);
}

void AFSoftSerial::print(unsigned long n)
{
  printNumber(n, 10);
}

void AFSoftSerial::print(long n, int base)
{
  if (base == 0)
    print((char) n);
  else if (base == 10)
    print(n);
  else
    printNumber(n, base);
}

void AFSoftSerial::println(void)
{
  print('\r');
  print('\n'); 
}

void AFSoftSerial::println(char c)
{
  print(c);
  println(); 
}

void AFSoftSerial::println(const char c[])
{
  print(c);
  println();
}

void AFSoftSerial::println(uint8_t b)
{
  print(b);
  println();
}

void AFSoftSerial::println(int n)
{
  print(n);
  println();
}

void AFSoftSerial::println(long n)
{
  print(n);
  println(); 
}

void AFSoftSerial::println(unsigned long n)
{
  print(n);
  println(); 
}

void AFSoftSerial::println(long n, int base)
{
  print(n, base);
  println();
}

// Private Methods /////////////////////////////////////////////////////////////

void AFSoftSerial::printNumber(unsigned long n, uint8_t base)
{
  unsigned char buf[8 * sizeof(long)]; // Assumes 8-bit chars.
  unsigned long i = 0;

  if (n == 0) {
    print('0');
    return;
  }

  while (n > 0) {
    buf[i++] = n % base;
    n /= base;
  }

  for (; i > 0; i--)
    print((char) (buf[i - 1] < 10 ? '0' + buf[i - 1] : 'A' + buf[i - 1] - 10));
}


I'd like to follow up and do a few more things with this library: (a) make it inherit from Print so that it is smaller, (b) revise it to support all pins to #19, and (c) complete the work that Tatien started to make it multi-instance capable.

Thanks again! Best wishes for the new year!

Mikal Hart
Intel Corporation
Last edited by mikalhart on Sun Jan 04, 2009 12:49 am, edited 1 time in total.
mikalhart
 
Posts: 18
Joined: Wed Jul 16, 2008 10:05 pm

Re: AFSoftSerial improvement

by adafruit on Sat Jan 03, 2009 9:44 pm

awsome. yes. its better to have a 'proper queue' but i was lazy :(

adafruit
 
Posts: 12151
Joined: Thu Apr 06, 2006 4:21 pm
Location: nyc

Re: AFSoftSerial improvement

by mikalhart on Sun Jan 04, 2009 12:18 am

...lazy


Oh, man, I understand being lazy... :) Somehow this little jewel of code has attracted my interest, though, so I don't feel too lazy about it.

Hopefully the addition of the "volatile" keyword and/or the circular buffer will resolve any mysterious problems that users have been having.

Thanks for the reply.

Mikal
mikalhart
 
Posts: 18
Joined: Wed Jul 16, 2008 10:05 pm

Re: AFSoftSerial improvement

by sanchoooo on Sun Jan 04, 2009 2:43 am

Cheers and Happy New Year,

For christmas I was given a Sanguino(ATMEGA644p) and I was trying to use AFSoftSerial to communicate with my Holux GM-210 gps. I found that the code for AFSoftSerial::begin was incorrect for the ATMEGA644p's registers. I wanted to pass the working(uncomplete) version I had to you for future incorporation into AFSoftSerial. I will complete the rest of the code for the remaining pins and repost the finished code.

I also added a SIGNAL for PIN_CHANGE1 and temp commented out the checks for the pins as I don't have the specifics on which signal gets called for which pins. I will clean this as well.

Code: Select all | TOGGLE FULL SIZE
SIGNAL(SIG_PIN_CHANGE0) {
//  if ((_receivePin >=8) && (_receivePin <= 13))
  {
    recv();
  }
}
SIGNAL(SIG_PIN_CHANGE1) {
//  if ((_receivePin >=8) && (_receivePin <= 13))
  {
    recv();
  }
}
SIGNAL(SIG_PIN_CHANGE2)
{
//  if (_receivePin <8)
  {
    recv();
  }
}



Code: Select all | TOGGLE FULL SIZE
void AFSoftSerial::begin(long speed)
{
  pinMode(_transmitPin, OUTPUT);
  digitalWrite(_transmitPin, HIGH);

  pinMode(_receivePin, INPUT);
  digitalWrite(_receivePin, HIGH);  // pullup!

  _baudRate = speed;
  switch (_baudRate) {
  case 115200: // For xmit -only-!
    _bitDelay = 4; break;
  case 57600:
    _bitDelay = 14; break;
  case 38400:
    _bitDelay = 24; break;
  case 31250:
    _bitDelay = 31; break;
  case 19200:
    _bitDelay = 54; break;
  case 9600:
    _bitDelay = 113; break;
  case 4800:
    _bitDelay = 234; break;
  case 2400:
    _bitDelay = 470; break;
  default:
    _bitDelay = 0;
  }   

   if ((_receivePin >=0) && (_receivePin <= 7)) {
      /*• Bit 7:0 – PCINT15:8: Pin Change Enable Mask 15..8
Each PCINT15:8-bit selects whether pin change interrupt is enabled on the corresponding I/O
pin. If PCINT15:8 is set and the PCIE1 bit in EIMSK is set, pin change interrupt is enabled on the corresponding I/O pin. If PCINT15:8 is cleared, pin change interrupt on the corresponding I/O pin is disabled.*/
     // PCINT8-15 = PINS 1-8(0-7) = PCICR = PCMSK1
     PCMSK1 |= (1<<_receivePin); //MSK1 NOT MSK2 FOR ATMEGA644P
       /*Bit 1 – PCIE1: Pin Change Interrupt Enable 1
When the PCIE1 bit is set (one) and the I-bit in the Status Register (SREG) is set (one), pin
change interrupt 1 is enabled. Any change on any enabled PCINT15..8 pin will cause an interrupt.
The corresponding interrupt of Pin Change Interrupt Request is executed from the PCI1
Interrupt Vector. PCINT15..8 pins are enabled individually by the PCMSK1 Register. */
     PCICR |= (1<<PCIE1); //Bit 1 – PCIE1: Pin Change Interrupt Enable 1
/*12.2.2 EIMSK – External Interrupt Mask Register
• Bits 2:0 – INT2:0: External Interrupt Request 2 - 0 Enable
When an INT2:0 bit is written to one and the I-bit in the Status Register (SREG) is set (one), the corresponding external pin interrupt is enabled. The Interrupt Sense Control bits in the External Interrupt Control Register, EICRA, defines whether the external interrupt is activated on rising or falling edge or level sensed. Activity on any of these pins will trigger an interrupt request even if the pin is enabled as an output. This provides a way of generating a software interrupt.*/
    EIMSK |= (1<<PCINT2);
    EIFR  |= (1<<INTF2);

  }

  whackDelay(_bitDelay*2); // if we were low this establishes the end
}


Sancho

sanchoooo
 
Posts: 1
Joined: Sun Jan 04, 2009 2:31 am

Re: AFSoftSerial improvement

by mikalhart on Wed Jan 21, 2009 7:55 pm

@ladyada --

I have made some more improvements to AFSoftSerial. Ultimately I'd like to post these on the Arduino.cc site, but I'd love to get your buy-in first, since your work with the interrupt mechanism is so key to this whole project. The changes I made are as follows:

1. Derived the class from class Print. This saves some 600+ bytes in duplicate code.
2. Added support for all pins 0-19 (0-21 on Arduino mini)
3. Renamed "whackDelay" to "tunedDelay".
4. Made the class multi-instance capable -- sort of**

That last item has a great big ** asterisk by it. I've come to realize that rewriting AFSoftSerial to reliably handle multiple instances, say 4 asynchronous devices receiving data at 38K baud, is a devilishly difficult, perhaps overwhelming, problem. Yet it is no doubt very useful to be able to support multiple serial devices simultaneously.

It occurred to me that a reasonable compromise solution would be to rewrite the library to support multiple objects, only one of which can be active at once. This will not be a perfect solution for all applications, of course, but I would be willing to bet that a large number would find such a design an improvement.

Consider, for example, a project that had a serial GPS, a serial thermometer, and a serial LCD. As long as you used the devices serially, you could build an application like this:

Code: Select all | TOGGLE FULL SIZE
#include <NewSoftSerial.h>

NewSoftSerial gps(4,3); // gps device
NewSoftSerial therm(6,5); // serial thermometer
NewSoftSerial LCD(8,7); // serial LCD

  ...
  read_gps_data(); // collect data from the GPS unit for a few seconds
  read_thermometer_data(); // collect temperature data from thermometer
  LCD.print("Data gathered..."); // LCD becomes the active device
  ...


Only one device can be active at a time. Calling available(), read(), print[ln](), or begin() on a particular port makes that the active one. One small benefit of this design is that it requires only one RX buffer shared between all instances.

What do you think of this design strategy? I don't expect it to be universally acclaimed, but I welcome your or anyone's feedback.

I have attached some sample code. Note that it does not incorporate Sanchoooo's suggestions.

Mikal
Attachments
NewSoftSerial.cpp
(6.57 KiB) Downloaded 466 times
NewSoftSerial.h
(2.36 KiB) Downloaded 440 times
mikalhart
 
Posts: 18
Joined: Wed Jul 16, 2008 10:05 pm

Re: AFSoftSerial improvement

by mikalhart on Mon Feb 02, 2009 3:38 pm

Hi, all--

I went ahead a formally posted my update to AFSoftSerial. See here and here.

I'm currently working on an update that supports 300, 1200, 14400, and 28800 baud, and am tuning the higher baud rates. Stay tuned.

Mikal
mikalhart
 
Posts: 18
Joined: Wed Jul 16, 2008 10:05 pm

Re: AFSoftSerial improvement

by mos6502 on Wed Feb 04, 2009 8:21 pm

Ive got a question regarding AFSoftwareSerial - what are the differences between this library and the SoftwareSerial thats already included in the IDE? Why would I use this library over the one thats already included?
mos6502
 
Posts: 25
Joined: Sun Nov 23, 2008 4:33 pm

Re: AFSoftSerial improvement

by mikalhart on Wed Feb 04, 2009 9:34 pm

Hi mos6502,

AFSoftSerial (and my derivative library NewSoftSerial) improve on SoftSerial in a very important way: they use interrupts to receive data and don't force the programmer to constantly listen for it. This is very important, especially with higher speed devices. If you use the built-in SoftSerial to read, say, GPS data from a 38400 baud unit, you have to call read() at least once every 26 microseconds or else risk losing data.

Here's a pretty good discussion that we've been having just today: http://www.arduino.cc/cgi-bin/yabb2/YaB ... 33753701/0.

Mikal
mikalhart
 
Posts: 18
Joined: Wed Jul 16, 2008 10:05 pm

Re: AFSoftSerial improvement

by mos6502 on Wed Feb 04, 2009 9:42 pm

Awesome - cleared everything up for me. Thanks A lot!
mos6502
 
Posts: 25
Joined: Sun Nov 23, 2008 4:33 pm

Re: AFSoftSerial improvement

by mtbf0 on Wed Feb 04, 2009 9:59 pm

did you by any chance add support for 8MHz operation? there was a guy asking about that on these forums a while back and after giving myself a headache trying to figure out how to calculate the constants for it, i just suggested that he select a baud rate twice what he really wanted. it worked, but support in the library would have been nice.
"i want to lead a dissipate existence, play scratchy records and enjoy my decline" - iggy pop, i need more
User avatar
mtbf0
 
Posts: 1645
Joined: Sat Nov 10, 2007 12:59 am
Location: oakland ca

Re: AFSoftSerial improvement

by mikalhart on Wed Feb 04, 2009 11:08 pm

Right now I'm doing baud rate tunings on the 16Mhz. There are some complaints that AF/New don't work great at 38.4K and higher, so I am seeing if I can't fix that. I've built a little test suite that helps automatically calculate those constants you refer to. Perhaps when this project is done, I can attack 8MHz operation, although I don't have any 8MHz Arduinos laying around.

Thanks for the note.

Mikal
mikalhart
 
Posts: 18
Joined: Wed Jul 16, 2008 10:05 pm

Re: AFSoftSerial improvement

by adafruit on Thu Feb 05, 2009 12:40 pm

the baud rate constants were actually done with a scope not calculated
they could theoretically change with each version of the IDE as optimization is tweaked
although, of course, it -shouldnt-...

adafruit
 
Posts: 12151
Joined: Thu Apr 06, 2006 4:21 pm
Location: nyc

Re: AFSoftSerial improvement

by mikalhart on Thu Feb 05, 2009 1:51 pm

they could theoretically change with each version of the IDE as optimization is tweaked


Hmm... I hadn't thought of that.

A guy on the Arduino forum was having trouble transmitting at 38.4K. By tweaking _bitDelay, he got it to work, but then RX was broken. I'm trying to massage it so that both RX and TX can work at 38.4 and 57.6. I should have something posted later today.

Mikal
mikalhart
 
Posts: 18
Joined: Wed Jul 16, 2008 10:05 pm

Re: AFSoftSerial improvement

by mtbf0 on Thu Feb 05, 2009 9:51 pm

hmmm.

and somehow it seems to turn out that for baud rate n
Code: Select all | TOGGLE FULL SIZE
bitrate(n*2) = bitrate(n) * 2 + 6
which is why i thought i could calculate values for 8MHz.

guess maybe a sketch that continuously varied bitrate until it could correctly receive and transmit a string of '7's at some baud rate.
"i want to lead a dissipate existence, play scratchy records and enjoy my decline" - iggy pop, i need more
User avatar
mtbf0
 
Posts: 1645
Joined: Sat Nov 10, 2007 12:59 am
Location: oakland ca

Re: AFSoftSerial improvement

by mikalhart on Fri Feb 06, 2009 2:03 pm

I posted a new version of NewSoftSerial. It now supports 14.4K and 28.8K and fine tunes the higher baud rates a little bit. On my system I now get error free TX at all bauds, 99.9% RX at 38.4K and 99.7% at 57.6K.

Arduino posting
NewSoftSerial download

To obtain the new tunings, I built an (overly?) elaborate test script that attempts to calculate bitdelays experimentally. It manually tries several different candidate values, sending and receiving 1000s of bytes with each, and selects the one that generates the fewest errors. I should be able to use the same system to automatically generate values for 8MHz chips, and I just remember that I have an Arduino Mini stashed away somewhere in my lab. Stay tuned! :)

Mikal
mikalhart
 
Posts: 18
Joined: Wed Jul 16, 2008 10:05 pm

Please be positive and constructive with your questions and comments.