Minimising coding

Hi forumers,

I have some code written to monitor key-presses. It is a bit clunky however it works as expected.
One thing that I can't seem to do is make the display appear steady. It is refreshing very fast (looks like rolling).
Is there a method for stabilising the display without compromising key-press capture speed?

Any pointers as how to minimise/streamline what I have here would be appreciated.

Thanks
Andy

sketch_nov07a.pde (2.06 KB)

You are clearing the display and up dating it every time round the loop. This causes the display to look flickery. Instead only clear and update the display when the thing being displayed actually needs to change.

int DOWNstate = 1;
int LEFTstate = 1;
int RIGHTstate = 1;
int PRESSstate = 1;
int UPstate = 1;

A couple of things here.
If you don't assign a value to globals like these, they will be initialised to zero.
Setting them to any other value, if zero will do, is a bit of a waste of program memory.
(anyone who can spell "nowt" must be interested in economy and parsimony, right? :wink: )
HOWEVER, there's no need for these variables to be globals anyway; you may as well declare them inside "loop".

Grumpy_Mike:
You are clearing the display and up dating it every time round the loop. This causes the display to look flickery. Instead only clear and update the display when the thing being displayed actually needs to change.

That makes a lot of sense. Thanks Mike, I'll work on it.

AWOL:

int DOWNstate = 1;

int LEFTstate = 1;
int RIGHTstate = 1;
int PRESSstate = 1;
int UPstate = 1;



A couple of things here.
If you don't assign a value to globals like these, they will be initialised to zero.
Setting them to any other value, if zero will do, is a bit of a waste of program memory.
(anyone who can spell "nowt" must be interested in economy and parsimony, right? ;-) )
HOWEVER, there's no need for these variables to be globals anyway; you may as well declare them inside "loop".

Understood AWOL.
Regarding declaring the variables to 1. The way the switch is wired, it is shorting to GND when pressed in any direction. So I need to set the digital inputs to active high, and the variable to 1 (HIGH)?

Let me play and see if they can be morphed.

Unless you're interested in the previous state of the switch (you're currently not), you don't have to initialise them at all, and they don't need to have global scope.
If you are interested in their previous state, you would give them global scope BUT initialise them with a "digitalRead" in "setup"

AWOL:
Unless you're interested in the previous state of the switch (you're currently not), you don't have to initialise them at all, and they don't need to have global scope.
If you are interested in their previous state, you would give them global scope BUT initialise them with a "digitalRead" in "setup"

Ok, many thanks for that, let me give it a try.

Grumpy_Mike:
You are clearing the display and up dating it every time round the loop. This causes the display to look flickery. Instead only clear and update the display when the thing being displayed actually needs to change.

Mike, I've tried a couple of different methods for clearing the disply but I'm getting poor results. The display either clears way too fast or not at all! Can you give me a pointer? Cheers.

Can you give me a pointer?

Sure. Post your code.

PaulS:

Can you give me a pointer?

Sure. Post your code.

No problem.

/*
Joystick-uLCD by Negativ3
Outputs the direction of the thumb stick to uLCD-144
*/

//Include the displayshield Library
#include <displayshield4d.h>

DisplayShield4d oled;

//Define pin assignments
const int DOWNPin = 2; const int LEFTPin = 3; const int RIGHTPin = 4;
const int PRESSPin = 5; const int UPPin = 6;

void setup() {

//Start serial comms
Serial.begin(57600);

//Initialise uLCD
oled.Init(); oled.Clear(); 

//Initialize the switch pins as a inputs:
pinMode(DOWNPin, INPUT);  digitalWrite(DOWNPin, HIGH);
pinMode(LEFTPin, INPUT);  digitalWrite(LEFTPin, HIGH);
pinMode(RIGHTPin, INPUT);  digitalWrite(RIGHTPin, HIGH);
pinMode(PRESSPin, INPUT);  digitalWrite(PRESSPin, HIGH);
pinMode(UPPin, INPUT);  digitalWrite(UPPin, HIGH);
}
void loop() {

//variables for reading switch status
int DOWNstate; int LEFTstate; int RIGHTstate; int PRESSstate; int UPstate;

oled.setfont(OLED_FONT5x7); oled.setfontmode(OLED_FONT_TRANSPARENT);

//read state of DOWN switch
DOWNstate = digitalRead(DOWNPin);
//check if DOWN switch is pressed
if(DOWNstate == LOW) {
        //write to LCD
        oled.drawstringblock(5, 5, 0, oled.RGB(255, 255, 0), 4, 4, "DOWN");
}
//read state of LEFT switch
LEFTstate = digitalRead(LEFTPin);
//check if LEFT switch is pressed
if(LEFTstate == LOW) {
        //write to LCD
        oled.drawstringblock(5, 5, 0, oled.RGB(255, 255, 0), 4, 4, "LEFT");
}
//read state of RIGHT switch
RIGHTstate = digitalRead(RIGHTPin);
//check if RIGHT switch is pressed
if(RIGHTstate == LOW) {
        //write to LCD
        oled.drawstringblock(5, 5, 0, oled.RGB(255, 255, 0), 4, 4, "RIGHT");
}
//read state of PRESS switch
PRESSstate = digitalRead(PRESSPin);
//check if PRESS switch is pressed
if(PRESSstate == LOW) {
        //write to LCD
        oled.drawstringblock(5, 5, 0, oled.RGB(255, 255, 0), 4, 4, "PRESS");
}
//read state of UP switch
UPstate = digitalRead(UPPin);
//check if UP switch is pressed
if(UPstate == LOW) {
        //write to LCD
        oled.drawstringblock(5, 5, 0, oled.RGB(255, 255, 0), 4, 4, "UP");
}
}

But you're still clearing it every time through 'loop'.

A simple wrapper function could do the clear and the update.

Putting two separate statements on a single line does help legibility.

AWOL:
But you're still clearing it every time through 'loop'.

A simple wrapper function could do the clear and the update.

Putting two separate statements on a single line does help legibility.

Nope, the LCD does not clear with the code above. It clears at void setup.

I will look into wrapper functions.

Are you being sarcastic regarding legibility? Hard to tell.

Sarcastic?
No, not this time.
Edit: oops that was supposed to say "doesn't". I apologise.

The IDE has a useful auto format tool, using ctrl-T

Why do you need to set the font every time through 'loop'?

The display either clears way too fast or not at all!

That is down to your library function.
How can it clear too fast? Do you mean without you seeing the previous display?
It could be you have contact bounce problems or it could be that the display is busy and not ready to accept the clear command. However without pulling apart the library you are using it is hard to tell.

(Revised)

Give this a go:

/*
Joystick-uLCD by Negativ3
Outputs the direction of the thumb stick to uLCD-144
*/

//Include the displayshield Library
#include <displayshield4d.h>

DisplayShield4d oled;

//Define pin assignments
const int DOWNPin = 2; const int LEFTPin = 3; const int RIGHTPin = 4;
const int PRESSPin = 5; const int UPPin = 6;

void setup()
{

//Start serial comms
Serial.begin(57600);

//Initialise uLCD
oled.Init(); oled.Clear(); 

//Initialize the switch pins as a inputs:
pinMode(DOWNPin, INPUT);  digitalWrite(DOWNPin, HIGH);
pinMode(LEFTPin, INPUT);  digitalWrite(LEFTPin, HIGH);
pinMode(RIGHTPin, INPUT);  digitalWrite(RIGHTPin, HIGH);
pinMode(PRESSPin, INPUT);  digitalWrite(PRESSPin, HIGH);
pinMode(UPPin, INPUT);  digitalWrite(UPPin, HIGH);
}

void loop()
{
//variables for reading switch status
int DOWNstate; int LEFTstate; int RIGHTstate; int PRESSstate; int UPstate;

oled.setfont(OLED_FONT5x7); oled.setfontmode(OLED_FONT_TRANSPARENT);

//read state of DOWN switch
if (DigitalRead(DOWNPin) == LOW)
{
  // check if already pressed
  if (DOWNstate == 0)
  {
        //write to LCD
        oled.drawstringblock(5, 5, 0, oled.RGB(255, 255, 0), 4, 4, "DOWN");
        DOWNstate = 1;
  }
} else {
          DOWNstate = 0;
         }

//read state of LEFT switch
if (digitalRead(LEFTPin) == LOW)
{
  // check if already pressed
  if (LEFTstate == 0)
  {
        //write to LCD
        oled.drawstringblock(5, 5, 0, oled.RGB(255, 255, 0), 4, 4, "LEFT");
        // set pressed
        LEFTstate = 1;
  }
} else {
          LEFTstate = 0;
         }

//read state of RIGHT switch
if (digitalRead(RIGHTPin) == LOW)
{
  // check if already pressed
  if (RIGHTstate == 0)
  {
        //write to LCD
        oled.drawstringblock(5, 5, 0, oled.RGB(255, 255, 0), 4, 4, "RIGHT");
        // set pressed
        RIGHTstate = 1;
  }
} else {
          RIGHTstate = 0;
         }

//read state of UP switch
if (digitalRead(UPPin) == LOW)
{
  // check if already pressed
   if (UPstate == 0)
  {
        //write to LCD
        oled.drawstringblock(5, 5, 0, oled.RGB(255, 255, 0), 4, 4, "UP");
        // set pressed
        UPstate = 1;
   }
} else {
          UPstate = 0;
         }

//read state of PRESS switch
if (digitalRead(PRESSPin) == LOW)
{
  // check if already pressed
  if (PRESSstate == 0)
  {
        //write to LCD
        oled.drawstringblock(5, 5, 0, oled.RGB(255, 255, 0), 4, 4, "PRESS");
        // set pressed
        PRESSstate = 1;
   }
} else {
          PRESSstate = 0;
         }

If you need button pulsing then add a timer and reset all the *state variables in the timer loop.

J

AWOL:
Sarcastic?
No, not this time.
Edit: oops that was supposed to say "doesn't". I apologise.

The IDE has a useful auto format tool, using ctrl-T

Why do you need to set the font every time through 'loop'?

Got it AWOL. The auto format is good.

The font needs to be set every time as this will turn out to be a specific level within a menu structure, so the font may change depending on the information needed to squeeze in.

Grumpy_Mike:

The display either clears way too fast or not at all!

That is down to your library function.
How can it clear too fast? Do you mean without you seeing the previous display?
It could be you have contact bounce problems or it could be that the display is busy and not ready to accept the clear command. However without pulling apart the library you are using it is hard to tell.

Ok Mike, it was happening when I put had oled.Clear() after the if statement was true. I thought it would clear the current display then display the text as formatted, but no. I think it was clearing too fast. Lib attached.

/*
	displayshield4d.h - Arduino Library for 4Display-Shield by 4d Systems
	Copyright(c) December 2010 Oscar Gonzalez - www.BricoGeek.com

	http://code.google.com/p/displayshield4d/

	Licensed under GNU General Public License v3 
	http://creativecommons.org/licenses/by/3.0/
	
	THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
	IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
	FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
	AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
	LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
	OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
	THE SOFTWARE.
*/

#ifndef _DISPLAYSHIELD4D_H_
	#define _DISPLAYSHIELD4D_H_

	#include <inttypes.h>

	#define OLED_RESETPIN			7

	#define OLED_BAUDRATE			9600
	#define OLED_RESETPIN			8  
	#define OLED_INITDELAYMS		1000

	#define	OLED_DETECT_BAUDRATE		0x55
	#define OLED_GETDEVICEINFO		0x56
	#define OLED_STRINGTERMINATOR		0x00

	#define	OLED_CLEAR			0x45

	#define OLED_SOLID			0
	#define OLED_WIREFRAME			1

	#define	OLED_COMMAND_CONTROL		0x59
	#define	OLED_COMMAND_DISPLAY_OFF	0x00
	#define	OLED_COMMAND_DISPLAY_ON		0x01
	#define	OLED_COMMAND_SHUTDOWN		0x00
	#define	OLED_COMMAND_POWEROFF		0x01
	#define OLED_COMMAND_SLEEP		0x5A
	#define OLED_COMMAND_STOP_SD		0x80
	#define OLED_COMMAND_WAKEONKOYSTICK	0x02
	#define OLED_COMMAND_WAKEONSERIAL	0x01
	#define OLED_SCREENCOPY			0x63

	#define OLED_ACK			0x06
	#define OLED_NAK 			0x15

	// Graphics
	#define	OLED_PUTPIXEL			0x50
	#define	OLED_READPIXEL			0x52
	#define	OLED_LINE			0x4C
	#define	OLED_SETBACKGROUND		0x42
	#define	OLED_SETPENSIZE			0x70
	#define	OLED_RECTANGLE			0x72
	#define	OLED_CIRCLE			0x43
	#define	OLED_TRIANGLE			0x47

	// Text
	#define	OLED_SETFONT			0x46
		#define	OLED_FONT5x7		0x00
		#define	OLED_FONT8x8		0x01
		#define	OLED_FONT8x12		0x02

	#define	OLED_SETFONTMODE		0x4F
		#define	OLED_FONT_TRANSPARENT	0x00
		#define	OLED_FONT_OPAQUE	0x01

	#define OLED_STRING_BLOCK		0x53

	// Class definition
	class DisplayShield4d 
	{
		public:
			DisplayShield4d();

			uint8_t Init();
			uint8_t Reset();
			uint8_t Clear();
			uint8_t GetReply();			
			/*
			char *GetDeviceType();
			uint8_t GetDeviceWidth();
			uint8_t GetDeviceHeight();
			*/
			unsigned int RGB(uint8_t red, uint8_t green, uint8_t blue);
			uint8_t SetPenSize(char val);
			uint8_t SetBackground(unsigned int color);
			uint8_t SetContrast(char val);
			uint8_t SetState(char state);
			uint8_t Sleep(char wake_cond);

			// Utility
			uint8_t ScreenCopy(uint8_t source_x, uint8_t source_y, uint8_t dest_x, uint8_t dest_y, uint8_t width, uint8_t height);

			// Graphics functions
			uint8_t putpixel(uint8_t x, uint8_t y, unsigned int color);
			uint16_t readpixel(uint8_t x, uint8_t y);
			uint8_t line(uint8_t x1, uint8_t y1, uint8_t x2, uint8_t y2, unsigned int color);
			uint8_t rectangle(uint8_t x, uint8_t y, uint8_t width, uint8_t height, char filled, unsigned int color);
			uint8_t circle(uint8_t x, uint8_t y, uint8_t radius, uint8_t filled, unsigned int color);
			uint8_t triangle(uint8_t x1, uint8_t y1, uint8_t x2, uint8_t y2, uint8_t x3, uint8_t y3, uint8_t filled, unsigned int color);

			// Text functions
			uint8_t setfont(uint8_t font_type);
			uint8_t setfontmode(uint8_t font_mode);
			uint8_t drawstringblock(uint8_t x, uint8_t y, uint8_t font, unsigned int color, uint8_t width, uint8_t height, char *text);


		private:
			//void GetDeviceInfo();

			uint8_t	device_type;
			uint8_t	device_hardware_rev;
			uint8_t	device_firmware_rev;
			uint8_t	device_width;
			uint8_t	device_height;

	};

#endif

j514:
(Revised)

Give this a go:

If you need button pulsing then add a timer and reset all the *state variables in the timer loop.

J

j514, ok I gave it a go, and looking at the way the code performs. The text overlays on the display. The display needs to be cleared when a new switch is pressed, before the new switch text is displayed. Interesting way of monitoring switch states. Thanks.

When a new button is pressed, just do a FillRect instead of clearing the whole screen. It will look much cleaner.

Regards,

J

j514:
When a new button is pressed, just do a FillRect instead of clearing the whole screen. It will look much cleaner.

Regards,

J

J, indeed it does. This is what the code looks like now.

/*
Joystick-uLCD by Negativ3
Outputs the direction of the thumb stick to uLCD-144
*/

//Include the displayshield Library
#include <displayshield4d.h>

DisplayShield4d oled;

//Define pin assignments
const int DOWNPin = 2; const int LEFTPin = 3; const int RIGHTPin = 4;
const int PRESSPin = 5; const int UPPin = 6;

void setup()
{

//Start serial comms
Serial.begin(57600);

//Initialise uLCD
oled.Init();
oled.Clear();

//Initialize the switch pins as a inputs:
pinMode(DOWNPin, INPUT);  digitalWrite(DOWNPin, HIGH);
pinMode(LEFTPin, INPUT);  digitalWrite(LEFTPin, HIGH);
pinMode(RIGHTPin, INPUT);  digitalWrite(RIGHTPin, HIGH);
pinMode(PRESSPin, INPUT);  digitalWrite(PRESSPin, HIGH);
pinMode(UPPin, INPUT);  digitalWrite(UPPin, HIGH);
}

void loop()
{
//variables for reading switch status
int DOWNstate; int LEFTstate; int RIGHTstate; int PRESSstate; int UPstate;

//read state of DOWN switch
if (digitalRead(DOWNPin) == LOW)
{
  // check if already pressed
  if (DOWNstate == 0)
  {
        //write to LCD
        oled.rectangle(0, 0, 128, 128, OLED_SOLID, oled.RGB(0, 0, 0));
        oled.setfont(OLED_FONT5x7);
        oled.setfontmode(OLED_FONT_TRANSPARENT);
        oled.drawstringblock(5, 5, 0, oled.RGB(255, 255, 0), 4, 4, "DOWN");
        DOWNstate = 1;
  }
} else {
          DOWNstate = 0;
         }

//read state of LEFT switch
if (digitalRead(LEFTPin) == LOW)
{
  // check if already pressed
  if (LEFTstate == 0)
  {
        //write to LCD
        oled.rectangle(0, 0, 128, 128, OLED_SOLID, oled.RGB(0, 0, 0));
        oled.setfont(OLED_FONT5x7);
        oled.setfontmode(OLED_FONT_TRANSPARENT);
        oled.drawstringblock(5, 5, 0, oled.RGB(255, 255, 0), 4, 4, "LEFT");
        // set pressed 
        LEFTstate = 1;
  }
} else {
          LEFTstate = 0;
         }

//read state of RIGHT switch
if (digitalRead(RIGHTPin) == LOW)
{
  // check if already pressed
  if (RIGHTstate == 0)
  {
        //write to LCD
        oled.rectangle(0, 0, 128, 128, OLED_SOLID, oled.RGB(0, 0, 0));
        oled.setfont(OLED_FONT5x7);
        oled.setfontmode(OLED_FONT_TRANSPARENT);
        oled.drawstringblock(5, 5, 0, oled.RGB(255, 255, 0), 4, 4, "RIGHT");
        // set pressed
        RIGHTstate = 1;
  }
} else {
          RIGHTstate = 0;
         }

//read state of UP switch
if (digitalRead(UPPin) == LOW)
{
  // check if already pressed
   if (UPstate == 0)
  {
        //write to LCD
        oled.rectangle(0, 0, 128, 128, OLED_SOLID, oled.RGB(0, 0, 0));
        oled.setfont(OLED_FONT5x7);
        oled.setfontmode(OLED_FONT_TRANSPARENT);
        oled.drawstringblock(5, 5, 0, oled.RGB(255, 255, 0), 4, 4, "UP");
        // set pressed
        UPstate = 1;
   }
} else {
          UPstate = 0;
         }

//read state of PRESS switch
if (digitalRead(PRESSPin) == LOW)
{
  // check if already pressed
  if (PRESSstate == 0)
  {
        //write to LCD
        oled.rectangle(0, 0, 128, 128, OLED_SOLID, oled.RGB(0, 0, 0));
        oled.setfont(OLED_FONT5x7);
        oled.setfontmode(OLED_FONT_TRANSPARENT);
        oled.drawstringblock(5, 5, 0, oled.RGB(255, 255, 0), 4, 4, "PRESS");
        // set pressed
        PRESSstate = 1;
   }
} else {
          PRESSstate = 0;
         }
}