Combining sketches- passing array variables to function

Projects General Guidance
1

I have 2 working VirtualWire receiver sketches I want to combine into one. One sketch receives 3 hex messages, the other 3 plain text messages assembly into longer arrays. Each line has 2 characters for identing so I'd like to look at those and direct the incoming to one of two functions.

Something like this:

#include <VirtualWire.h>

  byte replyB[66];
  char replyG[80];

void loop()
{
  uint8_t vBuf[VW_MAX_MESSAGE_LEN];                             
  uint8_t vBuflen = VW_MAX_MESSAGE_LEN;

  if (vw_get_message(vBuf, &vBuflen)) {
    for ( int i=0 ; i<=65 ; i++ ) {  
      if ((vBuf[0]==0xFA) && (vBuf[1]==0xAF)) {
        assembleB(i, vBuf[i], vBuflen[i]);
        }
      if ((vBuf[0]=='A') && (vBuf[1]=='_')) {
        assembleG(i, vBuf[i], vBuflen[i]);
        }
      }
      replyB = assembleB();
      replyG = assembleG();
    }
  }

/***************************************************************************************/

void assembleB(int i, uint8_t vBuf, uint8_t vBuflen) {
  byte replyB[66];
        if ((vBuf[0]==0xFA) && (vBuf[1]==0xAF) && (vBuf[2]==0x2) && (vBuf[3]==0x41)) {
//          (do stuff);
        }  
        return replyB; 
}

void assembleG(int i, uint8_t vBuf, uint8_t vBuflen) {
  char replyG[80];
        if ((vBuf[0]=='A') && (vBuf[1]=='_')) {
//          (do other stuff to assemble replyG);
        } 
        return replyG;  
}

This syntax doesn't compile, complaining about 'invalid types 'uint8_t[int]' for array subscript', as I don't know how to pass individual members of an array to a function. But while I figure that out my real question is whether this is a sensible approach, is there a better way to achieve what I'm trying to do?

For instance, should the functions have their own for loops to decode a whole vBuf message? That would (I think) mean using a common ident scheme and making all messages a fixed length, which means disturbing both transmitter sketches. Maybe that's worthwhile, or something else entirely, I'm guessing at the moment and would appreciate some guidance.

2

The code seems to be a muddle of several approaches and I'm not clear what it's trying to do.

I get the idea that you are receiving a buffer that is expected to contain either 0xFA and 0xAF or 'A' and '_' header followed by a message body, and you want to use the header bytes to identify the message and pass the message body to the corresponding function for processing. Does the buffer always contain a single message, or might it potentially contain multiple messages? Do you know whether the header is always at the start of the buffer? Could either of those header sequences appear within the body of the message, and if so how would you know whether they represent the start of a message?

The sort of thing you're trying to do should be easy to achieve one you are clear what behaviour you want.

3

I'm not surprised it's muddled, that's why I'm seeking advice :slight_smile:

Does the buffer always contain a single message, or might it potentially contain multiple messages?

I won't know until it's working but at the moment the input messages are of variable length and are sent randomly, so I'm expecting collisions. I think VirtualWire can discriminate between transmissions and discard noise- I've currently got both Tx going and the receiver properly ingests the messages for the sketch that's loaded.

I think I can implement fixed length messages with an End ident reasonably easily if I have to mess with the transmitter code.

Do you know whether the header is always at the start of the buffer?

The header is always the first 2 bytes transmitted. Based on that I'm hoping to divert the message to the appropriate code.

Could either of those header sequences appear within the body of the message, and if so how would you know whether they represent the start of a message?

Probably, but I think it's unlikely. If they do they'll cause a breakage but that will be trapped downstream as checksum errors and the info will be discarded. If the discard count is high I can change the idents to something more favourable.

4

Based on the header being the first two bytes transmitted, and only needing to process the first message in the buffer, yu could do it like this:

if (vw_get_message(vBuf, &vBuflen))
{
    if(vBuflen > 2)
    {
        if((vBuf[0]==0xFA) && (vBuf[1]==0xAF))
        {
            assembleB(i, &vBuf[2], vBuflen-2); // pass in the buffer less the two byte header
        }
        else if ((vBuf[0]=='A') && (vBuf[1]=='_'))
        {
            assembleG(i, &vBuf[2], vBuflen-2); // pass in the buffer less the two byte header
        }
        else
        {
            // unrecognised message header
        }
    }
}

void assembleB(uint8_t *vBuf, uint8_t vBuflen)
{
    if ((vBuf[0]==0x2) && (vBuf[1]==0x41))
    {
        whatever other parsing you want to do here ...
  ...
}

I'm not sure what the output of assembleB() and assembleG() will be, but if they are generating something that needs to be dealt with in some common way e.g. sending a response then you might want to pass in a pointer to the buffer where they should save the response, and a pointer to a variable to receive the length of the response, if that is variable.

5

@perplexed, can you post a complete sketch?

...R

6

Here you go, this receives from an Arduino that interrogates the gas boiler. The boiler sends out 64byte replies which have to be split into 3 for transmission over VW. I'm afraid the output lines are very long and I've had to truncate them to post, They're probably incomprehensible in a small code window anyway. They work though.

The other sketch is simpler, it just receives gas meter monitoring, but includes loads of debug info so I don't have to spend too much time in the cellar.

/************* Streaming *************/
template<class T> inline Print &operator <<(Print &obj, T arg) { 
  obj.print(arg); 
  return obj; 
}

/*************** VW ************/
#include <VirtualWire.h>
#include "SD.h"


byte reply[66];          
byte sum = 0;
byte csbyte;
byte cs = 0;
int w=0;      // various status conditions
int a=0;      // length of FAAF
int b=0;      // length of FBBF
int c=0;      // length of FCCF
int x=0;
long lastTime;

/*************** SD ************/
boolean sdOK = 0;                                             // SD operational
boolean datalogenable = 1;                                    // normally on
char name[] = "Log00.CSV";                                   
long oldFileSize=0;


void setup()
{

  // *** initialise VirtualWire
  vw_set_rx_pin(7);
  vw_set_ptt_inverted(true); 
  vw_setup(2000);	 
  vw_rx_start();     

  // *** initialise Serial    
  Serial.begin(115200); 
  Serial << freeRam() << "\nBoiler Monitor Rx start\n" ;

  // *** initialise SD  
  pinMode(4, OUTPUT);
  sdOK = SD.begin(4);                                         
  if(sdOK == 1){
    Serial << datalogenable << " SD GOOD";
  }
  if(sdOK == 0){
    Serial << datalogenable << " SD BAD";
    datalogenable = 0;
  }
  if(datalogenable == 1 && sdOK == 1){   
    Serial << "\nLogging to: " << (name) ;
    File dataFile = SD.open(name, FILE_WRITE);    
    dataFile <<  "\n************** Log start *************" ;
    dataFile.close();
  }
}


void loop()
{

  /***********************************************Receive******************************************/
  uint8_t vBuf[VW_MAX_MESSAGE_LEN];
  uint8_t vBuflen = VW_MAX_MESSAGE_LEN;

  if (vw_get_message(vBuf, &vBuflen)) 
  {

    /************************ debug raw reception*****************************
      Serial << "\nvBuf\t\t" ;
      for (int i = 0; i < vBuflen; i++) {
      Serial.print(vBuf[i], HEX);
      }
      Serial << "\t:" << vBuflen << "\n" ;
      
    /************************************** Assemble ***********************************************/
    for ( int i=0 ; i<=65 ; i++ ) {   
      if ((vBuf[0]==0xFA) && (vBuf[1]==0xAF) && (vBuf[2]==0x2) && (vBuf[3]==0x41)) {
        a = vBuflen-2;                                    
        if (i >= 0 && i <= vBuflen-3) {                   
          reply[i] = vBuf[i+2]; 
          w = 1;
        }
      }
      else {
        if ((vBuf[0]==0xFA) && (vBuf[1]==0xAF)) {
          Serial << "\nFailed A ";
        } 
      }
      if ( (vBuf[0]==0xFB) && (vBuf[1]==0xBF)) {        
        b = vBuflen-2;
        w = 2;
        if (i >= 0 && i <= vBuflen-3) {            
          reply[i+a]=vBuf[i+2]; 
        }
      }
      if ( (vBuf[0]==0xFC) && (vBuf[1]==0xCF) && vBuf[23]==0x3 ) {
        c = vBuflen-2;
        w = 3;
        if (i >= 0 && i <= vBuflen-2) { 
          reply[i+a+b]=vBuf[i+2]; 
        }
        x=1;
      }
      else {
        if ((vBuf[0]==0xFC) && (vBuf[1]==0xCF)) {
          Serial << "\nFailed C ";
        } 
      }
    } // for

    if ( w==3 && x==1) { 

      /***************************************** Checksum*****************************************/
      sum=0;
      for (int i = 0; i <= 63; i++) {       
        sum += reply[i];
      }
      csbyte = reply[65];                  // incoming checksum from Tx
      cs= csbyte - sum;

      if (cs != 0) {
        Serial << "\nBad Checksum";
        File dataFile = SD.open(name, FILE_WRITE);   
        dataFile << "\nBad Checksum";
        dataFile.close();
        debug2();                                       
      }
      else {
        //    Serial << "\nChecksum good  ";
        //    debug2();                      
      }
      x = 0;
    } // if w & x 


    /******************************************** Output *******************************************/
    if  (w == 3 && cs == 0 && datalogenable == 1 && sdOK == 1) {

      char value[250];                                      
      float result;


      /************************** HEX output ***************************************
      File dataFile = SD.open(name, FILE_WRITE);   
      dataFile << "\n"; 
      for ( int i=0 ; i<66 ; i++ ) { 
        Serial.print(reply[i], HEX);  
        dataFile.print(reply[i], HEX);
        dataFile << ",";
        Serial << " " ;
      }
      Serial << "\t:" << sizeof(reply);


      /**************************** SD Write ***************************************/
      dataFile << "\nBoiler," << reply[66-2] << ",date" << ",time," << reply[46]  << ","  << reply[47] << "," << reply[48] << ",";
      dataFile << (reply[6])  + (reply[7] << 8)  << ","  << (reply[8])  + (reply[9] << 8)  << ",,"  << (reply[14])  + (reply[15] << 8)  << ","  << (reply[12])  + (reply[13] << 8)  << ","  <<  (reply[33])  + (reply[34] << 8) << ","  << (reply[22])  + (reply[23] << 8)  << ","  << (reply[24])  + (reply[25] << 8)  << ","  << (reply[20])  + (reply[21] << 8)  << ","  << (reply[26])  + (reply[27] << 8)  << ","  << (reply[18])  + (reply[19] << 8)  << "," ;
      Serial   <<  "\nSD written: " << dataFile.size() - oldFileSize;   
      oldFileSize = dataFile.size();
      dataFile << "\nlooptime," << millis() - lastTime ;
      dataFile.close(); 
      
      /**************************** Print ***************************************/      
      Serial << "\tlooptime: " << millis() - lastTime << "\t";
      lastTime = millis();
      Serial   << "same as the datalog, truncated for posting";
      //******************************************** end Output **************************************************/ 
      w = 0;
    }                           // if w & cs  
  }                             // ifvBuf
}                               //loop



/**************************************************************************************************/
/* Free ram
/**************************************************************************************************/
int freeRam () {
  extern int __heap_start, *__brkval; 
  int v; 
  return (int) &v - (__brkval == 0 ? (int) &__heap_start : (int) __brkval); 
}

/**************************************************************************************************/
/* printhexbyte
/**************************************************************************************************/

void printhexbyte(byte x)
{
  Serial   <<  "0x";
  if (x < 16) {
    Serial   <<  '0';
  }
  Serial.print(x, HEX);
}

/**************************************************************************************************
 * debug1
 **************************************************************************************************/
void debug1(int w)
{
  Serial   << "\nReceived " ;
  if (w != 9) {
    Serial   << w ;
  }
  Serial   << ":\t" ; 
  for ( int i=0 ; i<66 ; i++ ) { 
    Serial.print(reply[i], HEX);  
    Serial   << " ";
  } 
  Serial << "\t:" << sizeof(reply);
}

/**************************************************************************************************
 * debug2
 **************************************************************************************************/
void debug2()
{
  Serial   << "\nReceived checksum byte = " << csbyte << "\nCalculated checksum    = " << sum << "\nerror                   = " << cs << "\n" ;
}


/**************************************************************************************************
 * debug3
 **************************************************************************************************
void debug3()
{
  Serial.print(F("\nconfidence w x: ")) ;     
  Serial.print(w) ; 
  Serial.print(x) ; 
  Serial.print(F("\nlength 1:\t")); 
  Serial.print(a+2); 
  Serial.print(F("\nlength 2:\t")); 
  Serial.print(b+2); 
  Serial.print(F("\nlength 3:\t")); 
  Serial.print(c+2); 
  Serial.print(F("\nlength full:\t")); 
  Serial.print(sizeof(reply)); 
  Serial.print("\nChecksum:\t");
  printhexbyte(sum);
}
7

As well as being very long and having an enormous amount of the code in loop() that code seems to be missing the function vw_get_message()

I sort of had the impression that function is at the heart of your query so it seems strange that it is omitted (but I may be wrong).

...R

8

vw_get_message() is a library function.

I'm sorry you think my code is too long, what would you cut without changing the functionality? Why is it a problem there's a lot in the loop?

More to the point, now you've seen it, how should I combine it with another sketch built on similar lines?

9

Based on the header being the first two bytes transmitted, and only needing to process the first message in the buffer, yu could do it like this:

Code:

if (vw_get_message(vBuf, &vBuflen))
{
if(vBuflen > 2)
{
if((vBuf[0]==0xFA) && (vBuf[1]==0xAF))
{
assembleB(i, &vBuf[2], vBuflen-2); // pass in the buffer less the two byte header
}
else if ((vBuf[0]=='A') && (vBuf[1]=='_'))
{
assembleG(i, &vBuf[2], vBuflen-2); // pass in the buffer less the two byte header
}
else
{
// unrecognised message header
}
}
}

void assembleB(uint8_t *vBuf, uint8_t vBuflen)
{
if ((vBuf[0]==0x2) && (vBuf[1]==0x41))
{
whatever other parsing you want to do here ...
...
}

I'm not sure what the output of assembleB() and assembleG() will be, but if they are generating something that needs to be dealt with in some common way e.g. sending a response then you might want to pass in a pointer to the buffer where they should save the response, and a pointer to a variable to receive the length of the response, if that is variable.

thanks for this, I've been puzzling about it, hence the delay in responding, but without much success.

Does the line "assembleB(i, &vBuf[2], vBuflen-2);" pass to the function a pointer to the third byte of vBuf array, and from that to the rest of the array, or does it pass the whole array contents? I'm not sure if it matters, except that vBuf needs to be ready for the next message pretty much immediately, so I've assumed incoming messages need to be copied elsewhere as soon as possible.

There's no 'for' loop, so I'm not sure what the use of 'i' is in this scheme, there's not really anything for it to count.

Also it doesn't compile:
core.a(main.cpp.o): In function main': C:\Portable Apps\arduino-1.0.5-r2\hardware\arduino\cores\arduino/main.cpp:11: undefined reference to setup'
which means nothing to me :frowning:

10

perplexed:
vw_get_message() is a library function.

I'm sorry you think my code is too long, what would you cut without changing the functionality? Why is it a problem there's a lot in the loop?

More to the point, now you've seen it, how should I combine it with another sketch built on similar lines?

I didn't mean your code is too long from your point of view - I just meant there an awful lot of it for me to plught plough through to find the specific piece that is relevant to your question.

I find that putting very little code in loop() and putting the rest of it if small functions with narrowly defined jobs and meaningful names makes the code much easier to understand. Ideally loop() has 6 or 8 lines that read like a book to explain the whole program. Then one can focus attention on whichever function is of interest and ignore all the code in the others. It also makes it simple to copy a function into a short test sketch that has none to the rest of the project to confuse things.

The demo in the first post of this Thread might give you an impression of the style I am talking about.

I will try to have another look at your code later.

...R

11

perplexed:
Also it doesn't compile:
core.a(main.cpp.o): In function main': C:\Portable Apps\arduino-1.0.5-r2\hardware\arduino\cores\arduino/main.cpp:11: undefined reference to setup'
which means nothing to me :frowning:

That just means that you didn't define a void setup() function in your sketch. The Arduino IDE requires each sketch to contain void setup() and void loop() functions.

The code fragment I posted passes a pointer to the message body in to the assembleB() and assembleG() functions. If you want to process the contents of the message body using a for loop inside those functions, you can do that.

12

My inclination would be to reorganize your code like below. I think you will see how it makes it easy to integrate the information from two sources.

// http://forum.arduino.cc/index.php?topic=253151.0

// all the usual definitions

byte gasBoilerData[70];
byte gasMeterData[70];

void setup() {
  // usual stuff

}

void loop() {
  getDataFromGasBoiler();
  getDataFromGasMeter();
  prepareBoilerDataForSDCard();
  prepareMeterDataForSDCard();
  saveDataToSDCard();
}

void getDataFromGasBoiler() {
  if (vw_get_message(vBuf, &vBuflen)) {
    for (byte n = 0; n <= 65; n++) {
      gasBoilerData[n] = vBuf[n];
    }
  }
}

void getDataFromGasMeter() {
  // something similar, presumably

}

void prepareBoilerDataForSDCard() {
  // haven't been able to figure out that stuff
}

void prepareMeterDataForSDCard() {

}

void saveDataToSDCard() {

}

By the way that "template" stuff at the top of your sketch seems to be useful as it allows Serial to be used in a way I am familiar with from Ruby. Where can I find out more about it?

...R

13

Wow, many thanks for that! It is the direction I was moving in when I hit the problems at the head of this thread, which PeterH kindly sorted with the correct syntax, but a lot clearer than I was. Also thanks for the link to your demonstration thread, which clarified other muddles I've been having.

As for the streaming print, I started using it to reduce memory usage- those big 'Serial << ' and 'dataFile << ' statements at the end were previously constructed using sprintf, with everything possible wrapped as SerialPrint(F("xx")) (as were more lines where the Hex is decoded, but which pushed the sketch size too big to post). This method uses about 1/3 less Ram and reduced the compile size by ~10%. It's also easier to read, write and edit, IMO, so there's little for me not to like except I haven't yet persuaded it to print Hex.

Some info is at Arduino Playground - StreamingOutput.

14

Thanks for the link.

...R