patterncMinor
Manage multiple file writes in C
Viewed 0 times
filewritesmultiplemanage
Problem
I have a very simple C program that continuously reads from a temperature sensor and displays the temperature on two 7-segment LEDs. Reading is simple, but to display the LEDs I need to write to 14 GPIO pins on every iteration. It gets real ugly.
I am running this program on an embedded device (BeagleBone Black), which makes certain aspects more of a concern. In general, would using array structure to manage files be more expensive in terms of memory and performance? Or is the difference trivial enough to be negligible, regardless of the number of files?
```
#include
#include
#include
#include
#include
// function to light up a segment.
// note that since the led has a common-anode design,
// a segment lights up when LOW is applied on its pin.
void printSeg(char* segFile) {
FILE* seg;
if ((seg = fopen(segFile, "r+")) != NULL) {
fwrite("0", sizeof(char), 1, seg);
fclose(seg);
}
}
// function to light off a segment.
// note that since the led has a common-anode design,
// a segment lights off when HIGH is applied on its pin.
void unprintSeg(char* segFile) {
FILE* seg;
if ((seg = fopen(segFile, "r+")) != NULL) {
fwrite("1", sizeof(char), 1, seg);
fclose(seg);
}
}
// function to light up the 7 segments of each LED accordingly to display the right temperature
void printTemp(int temperature) {
// path to all gpio pins used to control the 7 segments on each LED
char *leftAFile = "/sys/class/gpio/gpio66/value";
char *rightAFile = "/sys/class/gpio/gpio67/value";
char *rightFFile = "/sys/class/gpio/gpio68/value";
char *leftFFile = "/sys/class/gpio/gpio69/value";
char *leftEFile = "/sys/class/gpio/gpio45/value";
char *rightEFile = "/sys/class/gpio/gpio44/value";
char *leftBFile = "/sys/class/gpio/gpio23/value";
char *rightBFile = "/sys/class/gpio/gpio26/value";
char *leftGFile = "/sys/class/gpio/gpio47/
I am running this program on an embedded device (BeagleBone Black), which makes certain aspects more of a concern. In general, would using array structure to manage files be more expensive in terms of memory and performance? Or is the difference trivial enough to be negligible, regardless of the number of files?
```
#include
#include
#include
#include
#include
// function to light up a segment.
// note that since the led has a common-anode design,
// a segment lights up when LOW is applied on its pin.
void printSeg(char* segFile) {
FILE* seg;
if ((seg = fopen(segFile, "r+")) != NULL) {
fwrite("0", sizeof(char), 1, seg);
fclose(seg);
}
}
// function to light off a segment.
// note that since the led has a common-anode design,
// a segment lights off when HIGH is applied on its pin.
void unprintSeg(char* segFile) {
FILE* seg;
if ((seg = fopen(segFile, "r+")) != NULL) {
fwrite("1", sizeof(char), 1, seg);
fclose(seg);
}
}
// function to light up the 7 segments of each LED accordingly to display the right temperature
void printTemp(int temperature) {
// path to all gpio pins used to control the 7 segments on each LED
char *leftAFile = "/sys/class/gpio/gpio66/value";
char *rightAFile = "/sys/class/gpio/gpio67/value";
char *rightFFile = "/sys/class/gpio/gpio68/value";
char *leftFFile = "/sys/class/gpio/gpio69/value";
char *leftEFile = "/sys/class/gpio/gpio45/value";
char *rightEFile = "/sys/class/gpio/gpio44/value";
char *leftBFile = "/sys/class/gpio/gpio23/value";
char *rightBFile = "/sys/class/gpio/gpio26/value";
char *leftGFile = "/sys/class/gpio/gpio47/
Solution
Here are some things that may help you improve your program.
Use all required
The program uses
Use
Whenever you pass a pointer, ask yourself whether the called function should be allowed to modify the contents of the pointed-to memory. If not, then that parameter should be
Combine similar functions
The only difference between your
Separate calculation from I/O
The mapping of a value to a 7-segment display is common to both digits. The only thing that changes is the mapping of segments to file names. This suggests that a more compact way to approach this would be to use a two-level data structure. One structure would map from digit to segments and the other maps from segments to filenames. Here's test code I wrote to demonstrate the concept. Obviously, you'd need to use your actual file names and use the
Consider performance implications
Embedded systems work typically involves sometimes extreme limitations in terms of memory and processing speed that makes such work fundamentally different from writing software that runs on Windows or Linux or OS X, but a BeagleBone Black is a pretty high end embedded system, and so would be much closer to writing programs for an regular Linux workstation. For that reason, I'd recommend doing timing experiments to see if your update rate is fast enough. If not, one thing you might consider is opening the files once and keeping them open during the running of the program. Doing so would allow the
As you can see, you would now be passing file handles rather than opening and closing a file for each segment update. This is undoubtedly going to be faster -- the question is whether the increase in speed is worth the extra code that will have to be written to make sure all files are c
Use all required
#includesThe program uses
fopen but doesn't #include . It should.Use
const where practicalWhenever you pass a pointer, ask yourself whether the called function should be allowed to modify the contents of the pointed-to memory. If not, then that parameter should be
const. For example:void printSeg(const char* segFile) {Combine similar functions
The only difference between your
printSeg and unprintSeg is the value written. I think it would make more sense to pass that as a value:// note that since the led has a common-anode design,
// a segment lights off when HIGH is applied on its pin.
void setSeg(const char* segFile, bool val) {
FILE* seg;
if ((seg = fopen(segFile, "r+")) != NULL) {
fwrite(val ? "0" : "1", sizeof(char), 1, seg);
fclose(seg);
}
}Separate calculation from I/O
The mapping of a value to a 7-segment display is common to both digits. The only thing that changes is the mapping of segments to file names. This suggests that a more compact way to approach this would be to use a two-level data structure. One structure would map from digit to segments and the other maps from segments to filenames. Here's test code I wrote to demonstrate the concept. Obviously, you'd need to use your actual file names and use the
setSeg routine above rather than my toy version: void setSeg(const char* segFile, bool val) {
printf("%s = %d\n", segFile, val);
}
/*
* segments are defined in this order: A, F, B, G, E, C, D
* to make it easy to verify visually.
*/
const bool segment[][7] = {
/* 0 */ { 1,
1, 1,
0,
1, 1,
1 },
/* 1 */ { 0,
0, 1,
0,
0, 1,
0 },
/* 2 */ { 1,
0, 1,
1,
1, 0,
1 },
/* 3 */ { 1,
0, 1,
1,
0, 1,
1 },
/* 4 */ { 0,
1, 1,
1,
0, 1,
0 },
/* 5 */ { 1,
1, 0,
1,
0, 1,
1 },
/* 6 */ { 1,
1, 0,
1,
1, 1,
1 },
/* 7 */ { 1,
0, 1,
0,
0, 1,
0 },
/* 8 */ { 1,
1, 1,
1,
1, 1,
1 },
/* 9 */ { 1,
1, 1,
1,
0, 1,
0 },
/* A */ { 1,
1, 1,
1,
1, 1,
0 },
/* b */ { 0,
1, 0,
1,
1, 1,
1 },
/* C */ { 1,
1, 0,
0,
1, 0,
1 },
/* d */ { 0,
0, 1,
1,
1, 1,
1 },
/* E */ { 1,
1, 0,
1,
1, 0,
1 },
/* F */ { 1,
1, 0,
1,
1, 0,
0 }
};
/*
* segfiles are defined in this order: A, F, B, G, E, C, D
* to match with segment order
*/
const char *segfile[2][7] = {
{ "A", "F", "B", "G", "E", "C", "D" },
{ "a", "f", "b", "g", "e", "c", "d" }
};
void segs(int n)
{
int lo = n % 10;
int hi = n / 10;
for (int i=0; i < 7; ++i) {
setSeg(segfile[0][i], segment[hi][i]);
setSeg(segfile[1][i], segment[lo][i]);
}
}
int main()
{
segs(83);
}Consider performance implications
Embedded systems work typically involves sometimes extreme limitations in terms of memory and processing speed that makes such work fundamentally different from writing software that runs on Windows or Linux or OS X, but a BeagleBone Black is a pretty high end embedded system, and so would be much closer to writing programs for an regular Linux workstation. For that reason, I'd recommend doing timing experiments to see if your update rate is fast enough. If not, one thing you might consider is opening the files once and keeping them open during the running of the program. Doing so would allow the
setSeg routine to look like this:void setSeg(FILE* seg, bool val) {
fwrite(bool ? "0" : "1", sizeof(char), 1, seg);
}As you can see, you would now be passing file handles rather than opening and closing a file for each segment update. This is undoubtedly going to be faster -- the question is whether the increase in speed is worth the extra code that will have to be written to make sure all files are c
Code Snippets
void printSeg(const char* segFile) {// note that since the led has a common-anode design,
// a segment lights off when HIGH is applied on its pin.
void setSeg(const char* segFile, bool val) {
FILE* seg;
if ((seg = fopen(segFile, "r+")) != NULL) {
fwrite(val ? "0" : "1", sizeof(char), 1, seg);
fclose(seg);
}
}void setSeg(const char* segFile, bool val) {
printf("%s = %d\n", segFile, val);
}
/*
* segments are defined in this order: A, F, B, G, E, C, D
* to make it easy to verify visually.
*/
const bool segment[][7] = {
/* 0 */ { 1,
1, 1,
0,
1, 1,
1 },
/* 1 */ { 0,
0, 1,
0,
0, 1,
0 },
/* 2 */ { 1,
0, 1,
1,
1, 0,
1 },
/* 3 */ { 1,
0, 1,
1,
0, 1,
1 },
/* 4 */ { 0,
1, 1,
1,
0, 1,
0 },
/* 5 */ { 1,
1, 0,
1,
0, 1,
1 },
/* 6 */ { 1,
1, 0,
1,
1, 1,
1 },
/* 7 */ { 1,
0, 1,
0,
0, 1,
0 },
/* 8 */ { 1,
1, 1,
1,
1, 1,
1 },
/* 9 */ { 1,
1, 1,
1,
0, 1,
0 },
/* A */ { 1,
1, 1,
1,
1, 1,
0 },
/* b */ { 0,
1, 0,
1,
1, 1,
1 },
/* C */ { 1,
1, 0,
0,
1, 0,
1 },
/* d */ { 0,
0, 1,
1,
1, 1,
1 },
/* E */ { 1,
1, 0,
1,
1, 0,
1 },
/* F */ { 1,
1, 0,
1,
1, 0,
0 }
};
/*
* segfiles are defined in this order: A, F, B, G, E, C, D
* to match with segment order
*/
const char *segfile[2][7] = {
{ "A", "F", "B", "G", "E", "C", "D" },
{ "a", "f", "b", "g", "e", "c", "d" }
};
void segs(int n)
{
int lo = n % 10;
int hi = n / 10;
for (int i=0; i < 7; ++i) {
setSeg(segfile[0][i], segment[hi][i]);
setSeg(segfile[1][i], segment[lo][i]);
}
}
int main()
{
segs(83);
}void setSeg(FILE* seg, bool val) {
fwrite(bool ? "0" : "1", sizeof(char), 1, seg);
}Context
StackExchange Code Review Q#87800, answer score: 5
Revisions (0)
No revisions yet.