[CLOSED] reading 16-bit timer register correctly, and the timer mode

Thomas33:
I will post the final code, just to make sure that I haven't overseen anything, but it may take a few days until I can split of some time again.

Well - it took more than a month now - but as I promised: Here is the code. Unfortunately there is one compile error left, but i am sure some of you guys will help me out of that :slight_smile:

ADDED: The error is:
RS_Heartbeat.cpp: In function 'void __vector_17()':
RS_Heartbeat:97: error: redefinition of 'void __vector_17()'
RS_Heartbeat:53: error: 'void __vector_17()' previously defined here

/*
  roboshock heartbeat server
  
  - receives data from ethernet and sends it out via serial
  - requests data from serial all 20 ms and sends the response out via ethernet.
  
  If receving certain chars from ethernet at the beginning of a new string
  it takes special actions (and does not send such chars to serial)
 
 created 2012
 by Thomas Schuett
 */
// #define FASTADC 1

// defines for setting and clearing register bits
#ifndef cbi
#define cbi(sfr, bit) (_SFR_BYTE(sfr) &= ~_BV(bit))
#endif
#ifndef sbi
#define sbi(sfr, bit) (_SFR_BYTE(sfr) |= _BV(bit))
#endif



#include <SPI.h>
#include <Ethernet.h>
// #include "TimerOne.h"

// Enter a MAC address and IP address for your controller below.
// The IP address will be dependent on your local network:
byte mac[] = { 0xDE, 0xAD, 0xBE, 0xEF, 0xFE, 0xED };
byte ip[] = { 192,168,0, 111 };
//byte gateway[] = {192,168,0,1};	
//byte subnet[] = { 255, 255, 255, 0 };

volatile boolean fired = false;
 
int TIME_FOR_ONE_CMD = 10; // in ms, starting conservative, depends on bautrate, includes time for sending to eth
long startTime = 0;

// ethernet send data must be buffered, to let it all go into one ethernet packet
char buffer[200];

int instructions_sent = 0;
int left_time = 0;

// Initialize the Ethernet server library
// with the IP address and port you want to use 
// (port 80 is default for HTTP):
Server server(80);

Client client;

ISR(TIMER1_COMPA_vect)
{
  fired = true;
} 


void setup()
{
// from the forum:
#if FASTADC
  // set prescale to 16, speeds up analogRead by around 0.1 ms per read
  sbi(ADCSRA,ADPS2) ;
  cbi(ADCSRA,ADPS1) ;
  cbi(ADCSRA,ADPS0) ;
#endif

buffer[0] = 0;

  // start the Ethernet connection and the server:
  Ethernet.begin(mac, ip);
  server.begin();
  delay(1000);
  blink();
  
  Serial.begin(9600);
  Serial2.begin(9600);
  
//  Timer3.initialize(500000);         // initialize timer1, and set a 1/2 second period
//  Timer3.pwm(9, 512);                // setup pwm on pin 9, 50% duty cycle
//  Timer3.attachInterrupt(callback);  // attaches callback() as a timer overflow interrupt

  //Timer1::setMode (4, Timer1::PRESCALE_8, Timer1::NO_PORT);
  
  // CTC mode with clk/8 prescaler
   TCCR1A = 0;
   TCCR1B = 1<<WGM12 | 1<<CS11;
   
  TCNT1 = 0;         // reset counter
  OCR1A =  39999;       // compare A register value
  TIFR1 |= _BV (OCF1A);    // clear interrupt flag
  TIMSK1 = _BV (OCIE1A);   // interrupt on Compare A Match  

}

SIGNAL (TIMER1_COMPA_vect) 
{
 // handle_interrupts(_timer5, &TCNT5, &OCR5A); 
    // not needed in CTC mode: TCNT1 = 0; //  reset the timer 
    fired = true;
    TIFR1 = 1<<OCF1A;     // clear the Output Compare A Match Flag
}

// ================================================================================


void loop()
{
  if (fired)
    {
        fired = false;  

        startTime = millis();
        
        // we want to use the same connection over and over
        if (! client || ! client.connected()) { 
           client = server.available();
        }
        if (! client || ! client.connected()) { 
           return;
        }
        
        request_analog_from_serial();
        
        instructions_sent = 0;

        if (time_left() > TIME_FOR_ONE_CMD) {
          if (forward_one_instruction_from_ethernet())
             instructions_sent++;
        }
        
        if (time_left() > TIME_FOR_ONE_CMD) {
          if (forward_one_instruction_from_ethernet())
             instructions_sent++;
        }

        if (fired) {
          // bad_timing ...
          left_time= -1;
        }
        else {
          // check_time_left ...
          left_time= time_left();        
        }
        
        sprintf(buffer+strlen(buffer),"Commands sent=%d\n", instructions_sent);
        sprintf(buffer+strlen(buffer),"Time left=%d\n", left_time);
        
        bufferToEthernet();
    }
} // end of loop
    
long time_left() {
  long now = millis();
   return (20 - (now - startTime));
}  
    
boolean forward_one_instruction_from_ethernet() {  

  if (! client.available()) { return false; }
        
  char c = client.read();

       if (c == 'V') { // needs 10 to 11 ms for all 16 analogValues :-)
          char a[170];
          a[0]=0;
          for (int analogChannel = 0; analogChannel < 16; analogChannel++) {
            sprintf(a+strlen(a),"a%02d=%d\n",
              analogChannel,analogRead(analogChannel));
          }    
          client.write(a);
          client.println("END.");
        }

        else if (c == 'X') {
          return false;
        }

        else if (c == 'H') {
          Serial2.begin(38400);
        }

        else if (c == 'L') {
          Serial2.begin(9600);
        }

        else {
          while (c != '\n') {
            Serial.print(c);
            Serial2.print(c);
            while (! client.available()) {
            }
            c = client.read();
          }
          Serial2.print("\r");    //  !!!  \r  !!!
          Serial.print("\n");
        }
        
  return true;
}


void request_analog_from_serial() {
  Serial2.print("5a\r\n");
  serialToBuffer();
}



void serialToBuffer() {
  int i;
  // have a timeout
  long start = millis();
  while (true) {
     if (Serial2.available()) {
        i = strlen(buffer);
        char c2 = Serial2.read();
        buffer[i] = c2;
        buffer[i+1] = 0;
        if (c2 == '\n') {
           return;
        }
      }
    
      long now = millis();
      if ((now - start) > 100) {
         buffer[i] = 0;
         return;
      }
      
      if (strlen(buffer) > 168) {
        return;
      }
  }
}


boolean bufferToEthernet() {
   if (client.connected()) {
         client.write(buffer);
         buffer[0] = 0;
         return true;
   }
   return false;
}


  void blink() {
          digitalWrite(13, HIGH);   // set the LED on
      delay(200);              // wait for a second
      digitalWrite(13, LOW);    // set the LED off
      delay(500);              // wait for a second
  }
  void blink2() {
          digitalWrite(13, HIGH);   // set the LED on
      delay(100);              // wait for a second
      digitalWrite(13, LOW);    // set the LED off
      delay(100);              // wait for a second
  }

If you have any remarks on the code, please feel free to mention it.

And if you want to know, for what all this shall be good for, you are welcome to visit my (recently ennewed) project page at roboshock.de :slight_smile:

At the two line numbers given:

ISR(TIMER1_COMPA_vect)
{
  fired = true;
}

...

SIGNAL (TIMER1_COMPA_vect) 
{
 // handle_interrupts(_timer5, &TCNT5, &OCR5A); 
    // not needed in CTC mode: TCNT1 = 0; //  reset the timer 
    fired = true;
    TIFR1 = 1<<OCF1A;     // clear the Output Compare A Match Flag
}

From interrupts.h:

SIGNAL: This is the same as the ISR macro without optional attributes.
deprecated Do not use SIGNAL() in new code. Use ISR() instead.

So those two functions are effectively doing the same thing. They are interrupt handlers for vector 17 (TIMER1_COMPA_vect).

Oh yes, i didn't even noticed the two routines. Now I removed the first and renamed the 2nd. Thanks Nick!

I already get the next error:
RS_Heartbeat.cpp.o: In function __static_initialization_and_destruction_0': RS_Heartbeat.cpp:62: undefined reference to Client::Client()'

My problem is, that the line numbers seem useless to me. The arduino ide (.22) doesn't even show line numbers, and I believe that it would not be usefull anyway because of the precompiler. The line numbers refer to the arduino precompiler output, does it? Where can i find it, or how can I utilize the given line numbers?

They compensate for that in the IDE. Look down at the bottom LH corner of the IDE window. The number there is the line number the cursor is on. Or just copy the whole thing into another text editor, one that has a "go to line" feature, and find it that way.

hmm - in line 62 there is:
cbi(ADCSRA,ADPS0) ;

but I don't think this is related at all to the error message?
RS_Heartbeat.cpp:62: undefined reference to `Client::Client()'

Can you post your amended code please?

It is exactly the code above, but removed the "ISR" method (one empty line plus/minus shouldn't make it unreproducible in principel) and renamed the "SIGNAL" method to "ISR" (without moving it).

I wonder, why the error line numbers are wrong. I am using ide .22 version under linux, if this may be of interest.

Can it be that my code line
Client client;
produces the error message
undefined reference to `Client::Client()'

Can it be, that in C (I am not a C programmer, sorry) such a line is an implicit construction of a Client object already? And imho there is no such constructor, as a client object will only be created by a server.available() call? If so, how can I just declare the variable of type Client, without constructing it? I think I need it, because the client object shall survive multiple calls of run().

BTW: Thanks for sticking to this topic!

Can it be that my code line
Client client;
produces the error message
undefined reference to `Client::Client()'

Yes. That line is creating an instance of the Client class, which requires executing the constructor.

If so, how can I just declare the variable of type Client, without constructing it?

You don't. You can create a pointer to an instance of type Client, and then point that pointer at an existing instance.

PaulS:

If so, how can I just declare the variable of type Client, without constructing it?

You don't. You can create a pointer to an instance of type Client, and then point that pointer at an existing instance.

Oh, yes. I see, it's C. So I need a pointer ;->
So I should try "Client* client;" or similar, right? I'll try it this evening (europe time), but thanks already!

Client *pClient;

will define a variable that is a pointer to a Client. That pointer does NOT point to anything, yet, so do NOT try to dereference it.

Sorry for bothering again, but as I told before, I am not a C programmer.
After changing the code to using a pointer for client, I don't know, which version is right:

Client *client;
a) if ((client == 0) || ! client->connected()) {
b) if ((*client == 0) || ! client->connected()) {
(... or completely different?)

Both compile, but fiddling this out with try&error is a bit to much walking on thin ice, so your advice would be appreciated.

The assignment
*client = server.available();
should be all right, as I found no other compiling combination, right?

The assignment
*client = server.available();
should be all right, as I found no other compiling combination, right?

It looks OK. What's the problem? If there is a problem, where is (ALL) the code?

a) if ((client == 0) || ! client->connected()) {

Looking at this tiny snippet (hint) I am guessing the intention is:

  • If client is not NULL (0) then we can dereference it (otherwise it would crash)
  • If we can dereference client, do so and see if "connected()" returns true.

as I found no other compiling combination, right?

Fiddling with code to make it compile, but with no other motivation or understanding, is likely to simply make it crash at runtime. Don't just add or subtract asterisks to get rid of errors. Look at the documentation and work out what is correct. It could be that the variable (eg. client) is not declared correctly, and by adding an asterisk you now have two mistakes rather than one.

Full ACK. (As I wrote before too.)

I go for a different approach now.

void loop()
{
  Client client = server.available();  // non blocking!
  if (client) {
     newConnectionCount++;
  
     while (client.connected()) { 
    
        char buffer[170];
        buffer[0] = 0;
        
        sprintf(buffer+strlen(buffer),"newConnectionCount=%d\n", newConnectionCount);

        startTime = millis();
        sprintf(buffer+strlen(buffer),"Starttime=%ld\n", startTime/1000);

         client.println(buffer);
         client.flush();
         
        while (! fired) { }  // waits for the next timer interrupt
        fired=false;         // clears the timer interrupt marker
        
    } 
  
    client.stop();
  }
  delay(100);
}

This will do it and keeps me away from these pointer issues.

I will mark this thread as CLOSED now, as the initial questons are answered already, and come up with another issue around this in a new thread.

Thank you!