Important changes to forums and questions
All forums and questions are now archived. To start a new conversation or read the latest updates go to forums.mbed.com.
7 years, 10 months ago. This question has been closed. Reason: Off Topic
Help with writing my first library
Hello to all, i am writing my first library and i did look at Writing a Library - Cookbook | Mbed but i still don t understand what i m doing wrong. My library function called "led" should turn on one of three LEDs connected to digital output pins, if i send 0 it lights up 1. led , if i send 1 it lights up 2. led and so on.
In the comment i have written all of the errors that i get, please help, Thank you in advance!
.h file
#ifndef midi_control
#define midi_control
#include "mbed.h"
class midi_control { //declaration does not declare anything (it is a warning not an error)
public:
midi_control(PinName led1,PinName led2,PinName led3); // "error expected a ;"
void led(int x);
private:
DigitalOut _led1;
DigitalOut _led2;
DigitalOut _led3;
};
#endif
.cpp file
#include "mbed.h"
#include "midi_control.h"
midi_control::led(PinName led1,PinName led2,PinName led3) : _led1(led1),_led2(led2),_led3(led3)
void midi_control::led(int x) {
if (x==0) {
_led1=1;
_led3=0;
}
else if (x==1){
_led2=1;
_led1=0;
}
else{
_led3=1;
_led2=0;
}
}
And here is main program :
#include "mbed.h"
#include "midi_control.h"
midi_control light(PTB8,PTB9,PTB10); // "expected a )"
int main() {
light.led(1); // "expression must have class type"
}
2 Answers
7 years, 10 months ago.
Not related to the compile errors but I think your led() function has some logical flaws. e.g. if you have the pattern led(0);led(2); you will end up with LEDs 1 and 3 on. If you have led(1);led(2); you will get just LED 3 on.
I'm guessing this probably isn't correct, fortunately it's easy to fix, you just need to specify each LED in each situation. Also you can use a switch rather than a series of else if lines, it makes the code look a little cleaner e.g.
void midi_control::led(int x) {
switch(x) {
case 0:
_led1=1;
_led2=0;
_led3=0;
break;
case 1:
_led1=0;
_led2=1;
_led3=0;
break;
case 2:
_led1=0;
_led2=0;
_led3=1;
break;
default: // any other value and they all go off.
_led1=0;
_led2=0;
_led3=0;
break;
}
}
Or if you want to get clever you could use a BusOut rather than three DigitalOuts, a little less intuitive but your led() function becomes a single line for anything up to 32 leds. For 3 LEDs setting each one individually isn't that much of a pain but if you ever need more than 3 then this approach is far nicer.
class midi_control {
[...]
private:
BusOut _leds;
};
midi_control::led(PinName led1,PinName led2,PinName led3) : _leds(led1,led2,led3) {
_leds=0; // turn all LEDs off on startup
}
void midi_control::led(int x) {
_leds=1<<x;
}
7 years, 10 months ago.
Hi,
You defined "midi_control" to NOTHING at the beginning of the header.
Changing the 2 lines as
#ifndef _midi_control_ #define _midi_control_ ...
will take care of the problem
Meantime, in the cpp file, line4 should be
midi_control::midi_control(...
And last not least, probably PTB8, PTB9, PTB10 will not work as PwmOut, so use pins which supports PwmOut or just use DigitalOut.
moto
Hi, This is my trial.
midi_control.h
#ifndef _midi_control_h_
#define _midi_control_h_
#include "mbed.h"
class midi_control {
public:
midi_control(PinName led1, PinName led2, PinName led3);
~midi_control() ;
void led(int x);
private:
DigitalOut _led1;
DigitalOut _led2;
DigitalOut _led3;
};
#endif
Since LEDs here are negative logic, I assigned initial value as 1 to turn them off.
midi_control.cpp
#include "midi_control.h"
midi_control::midi_control(PinName led1,PinName led2,PinName led3) :
_led1(led1, 1),_led2(led2, 1),_led3(led3, 1)
{
}
midi_control::~midi_control()
{
}
void midi_control::led(int x)
{
if (x==0) { /* trun on LED1 */
_led1=0;
_led2=1;
_led3=1;
} else if (x==1){ /* turn on LED2 */
_led1=1;
_led2=0;
_led3=1;
} else if (x==2){ /* turn on LED3 */
_led1=1;
_led2=1;
_led3=0;
} else { /* turn off all LEDs */
_led1=1 ;
_led2=1 ;
_led3=1 ;
}
}
main.cpp
#include "mbed.h"
#include "midi_control.h"
// midi_control light(PTB8,PTB9,PTB10); // "expected a )"
/* for FRDM-KL25Z */
midi_control light(PTB18,PTB19,PTD1);
int main() {
int i ;
while(1) {
light.led(i);
i = (i + 1) % 3 ; /* make i as 0, 1, 2, 0, 1, .. */
wait(0.5) ;
}
}
moto
posted by 26 Jan 2018
Thank you all for your help! I figured it out with your help and help from friend, now it works as i wanted to. I am very tankful for your answers, good to see this is friendly and helpful community. Andy A the logic is good because function led in main program will always take int values from 0 to 2 (0,1,2) and then again from 0 to 2, it will never go from 0 to 2 or something like that, but i understand what you wanted to say. Once more, many thanks! :)
posted by Karlo Obad 26 Jan 2018