Hello everyone, thanks for taking a look!
I am working on what is essentially a library for an MPC7940 real time clock chip to be used in a different project.
I am attempting to create a structure within the arduino to communicate with processing over serial and then send the data to the RTC. The RTC is accessed by the arduino over I2C,
I have the chip communicating nicely with my arduino and I can even send a nice 11 byte unix epoch time code and watch the RTC chip update.
All is well with that I hope other people might find that part of the code helpful.
For debigging (lol debugging) purposes, I would like to add several other commands to read a few other registers on the RTC such as to check the oscillator status or manipulate the alarms. My problem exists in the processSyncMessage() function, as I dont know how to filter serial messages based on their length. I am very new to programming so I wont be suprised at all if while loops are a terrible way of accomplishing this task.
What I want: To be able to send codes of different lengths and give the arduino a threshold to differentiate between them.
I.e. serial messages > 5 triggers someFunction(), serial messages < 5 triggers someOtherFunction()
**What happens:**I can only send one command and then the while loops get confused and wont respond to the commands. I can succesfully send a command under or over the threshold, however only one before I have to reset the arduino.
I think what I am looking for is some way of clearing the serial data so the length of the previous command isn't included in the next command?
Anyways, thanks for any help, if I find the answer before anyone responds I will post the solution in case anyone else is having a problem similar to this.
tip: improve your coding style both Arduino and Processing have a CTRL-T autoformatting function. That makes code more readable by indenting in a consistent way. Also to much empty lines between functions, decrease readability.
The first error is already in popup
void setup()
{
Serial.println("begin"); // <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<< what baudrate should it use?
Wire.begin(); // join i2c bus (address optional for master)
Serial.begin(9600); // start serial for output
Oooh, Good catch. Thanks for the advice!
I admit this is very messy, this is maybe my third sketch so I am not familiar most of the conventions. All that white space was supposed to make is easier for my brain to process this in chunks..
Don't use comments until they explain something properly'
#define month_mask 31 //for discarding leap year bit
#define sec_mask 127 //for discarding leap year bit
copy paste?
refactored your first chunk ( a bit) with some comments, just to get a feeling of how style can help (it is definitely not 100% yet)
Note I did not restructure the code itself except for the switch() instead of multiple if statements.
// use a header to have some meta information for version control etc.
//
// FILE: .ino
// AUTHOR:
// VERSION: 0.1.
// PURPOSE:
// DATE: 2013-12-...
// URL:
//
// Released to the public domain
//
#include <Time.h>
#include <Wire.h>
int rtc_address = 0x6F;
// RTC communication constants
#define TIME_MSG_LEN 11
#define TIME_HEADER 'T' ; // Header tag for serial time sync message
#define TIME_REQUEST 7 // ASCII bell character requests a time sync message
// 'standard' states that #defines use CAPITALS like above
// RTC masks
#define time_format 63 // bit 6: 0 = 24 hour format, 1 = 12 hour <<< good comment.
#define month_mask 31
#define sec_mask 127
#define osc_on 128
#define first_five 7
void setup()
{
Serial.begin(9600); // use 115200 if possible; is 12x as fast
Serial.println("Begin");
Wire.begin();
Serial.println("test time sent");
read_Time();
Serial.println("----read test----");
}
//
// explain purpose of function here
//
long processSyncMessage()
{
const char timeHeader = TIME_HEADER;
char osc_test = 'O';
char reload_header = 'R';
char stop_osc = 'S';
char set_alarm = 'A';
// if time sync available from serial port, update time and return true
while (Serial.available() >= TIME_MSG_LEN ) // time message consists of header & 10 ASCII digits
{
char c = Serial.read() ;
Serial.print(c);
Serial.println("-header");
if ( c == timeHeader )
{
Serial.println("headed");
time_t pctime = 0;
for (int i=0; i < TIME_MSG_LEN -1; i++)
{
c = Serial.read();
if ( c >= '0' && c <= '9')
{
pctime = (10 * pctime) + (c - '0') ; // convert digits to a number
}
else
{
// ??? error ???
}
}
setTime(pctime);
set_Time();
delay(100);
}
Serial.flush();
}
while (Serial.available() == 2 ) // VERY dangerous as one extra char would let this fail..
{
char command = Serial.read() ; // renamed b to command as that tells more about
Serial.print(b);
Serial.println("-header");
switch (command ) // this is the way to handle multiple if statements
{
case reload_header:
Serial.println("rereading");
read_Time();
break;
case test:
int oscBit = readFunction(0x03) & 32;
if (oscBit >= 1) Serial.println("running");
else Serial.println("stopped");
break;
case stop_osc:
writeFunction(0x00, 0);
int oscBit = readFunction(0x03) & 32;
if (oscBit < 1) Serial.println("stopped");
else Serial.println("running");
break;
case set_alarm:
setAlarm(2);
Serial.println("Alarm set to");
break;
default:
break;
}
}
}
Again thanks for the tips! I see why sometimes it is frustrating finding documentation for software, the software is the documentation.
The switch case worked almost perfectly, however I had to add some brackets for each case to keep it from getting confused. Now it seems like all of the bytes are properly handled as well because the while loop successfully pauses the loop() function when there is no serial available. Am I correct in thinking that no loop() progress means no left over serial bytes?
robtillaart:
while (Serial.available() == 2 ) // VERY dangerous as one extra char would let this fail..
What do you think would be a safer alternative? Longer command codes perhaps?
Heres what I changed it to in case anyone is curious:
long processSyncMessage()
{
char timeHeader = TIME_HEADER;
const char osc_test = 'O';
const char reload_header = 'R';
const char stop_osc = 'S';
const char set_alarm = 'A';
// if time sync available from serial port, update time and return true
while (Serial.available() >= TIME_MSG_LEN ) // time message consists of header & 10 ASCII digits
{
char c = Serial.read() ;
Serial.print(c);
Serial.println("-header");
if ( c == timeHeader )
{
Serial.println("headed");
time_t pctime = 0;
for (int i=0; i < TIME_MSG_LEN -1; i++)
{
c = Serial.read();
if ( c >= '0' && c <= '9')
{
pctime = (10 * pctime) + (c - '0') ; // convert digits to a number
}
else
{
// ??? error ???
}
}
setTime(pctime);
set_Time();
delay(100);
}
Serial.flush();
}
while (Serial.available() == 2 ) // VERY dangerous as one extra char would let this fail..
{
char command = Serial.read() ; // renamed b to command as that tells more about
Serial.print(command);
Serial.println("-header");
switch (command ) // this is the way to handle multiple if statements
{
{
case reload_header:
Serial.println("rereading");
read_Time();
break;
}
{
case osc_test:
int oscBit = readFunction(0x03) & 32;
if (oscBit >= 1) Serial.println("running");
else Serial.println("stopped");
break;
}
{
case stop_osc:
writeFunction(0x00, 0);
int oscBit = readFunction(0x03) & 32;
if (oscBit < 1) Serial.println("stopped");
else Serial.println("running");
break;
}
{
case set_alarm:
setAlarm(2);
Serial.println("Alarm set to");
break;
}
{
default:
break;
}
}
}
}
Your code is wasting a lot of SRAM unnecessarily. Using the F() macro would stop that.
Serial.println(F("-header"));
What do you think would be a safer alternative? Longer command codes perhaps?
while (Serial.available() >= 2 )
If you get the header ("T"), there is NO guarantee that the other 10 characters are available to be read. Your code assumes that they are. That's a very bad assumption.
PaulS:
Your code is wasting a lot of SRAM unnecessarily. Using the F() macro would stop that.
Thanks, I didn't even know about that.
If you get the header ("T"), there is NO guarantee that the other 10 characters are available to be read. Your code assumes that they are. That's a very bad assumption.
Don't I need to have at least 11 to get the T header in the first place?
Everything as it is is working well now, now I am trying to figure out the correct bits to set to configure the alarm to set the Multi Function Pin high.
