Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Various cleanups #23

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 13 additions & 24 deletions bitlash.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,27 +34,16 @@


***/
#if defined(ARDUINO) && ARDUINO >= 100
#include "Arduino.h"
#else
#include "WProgram.h"
#endif

#ifdef UNIX_BUILD
#include "src/bitlash-unix.c"
//#else
//#include "src/bitlash-arduino.c"
#endif

#include "src/bitlash-cmdline.c"
#include "src/bitlash-eeprom.c"
#include "src/bitlash-error.c"
#include "src/bitlash-functions.c"
#include "src/bitlash-builtins.c"
#include "src/bitlash-interpreter.c"
#include "src/bitlash-instream.c"
#include "src/bitlash-parser.c"
#include "src/bitlash-serial.c"
#include "src/bitlash-taskmgr.c"
#include "src/bitlash-api.c"
#include "src/eeprom.c"

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Support for Arduino pre-1.0 cannot be dropped, for backwards compatibility with existing projects.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This particular change does not drop pre-1.0 support, it just removes these includes from bitlash.h in favor of src/bitlash.h (I was planning to drop pre-1.0 support for my upcoming pullrequest, but let's discuss this separately there).

#include "src/bitlash-cmdline.cpp"
#include "src/bitlash-eeprom.cpp"
#include "src/bitlash-error.cpp"
#include "src/bitlash-functions.cpp"
#include "src/bitlash-builtins.cpp"
#include "src/bitlash-interpreter.cpp"
#include "src/bitlash-instream.cpp"
#include "src/bitlash-parser.cpp"
#include "src/bitlash-serial.cpp"
#include "src/bitlash-taskmgr.cpp"
#include "src/bitlash-api.cpp"
#include "src/eeprom.cpp"
13 changes: 10 additions & 3 deletions src/Makefile
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
all:
gcc -pthread *.c -o bin/bitlash
CFLAGS := -DUNIX_BUILD -pthread
LDFLAGS := -lstdc++ -pthread
SOURCES := $(wildcard *.cpp)
BINARY := bin/bitlash
PREFIX := /usr/local
BINPATH := $(PREFIX)/bin

all:
gcc -DUNIX_BUILD $(LDFLAGS) $(CFLAGS) $(SOURCES) -o $(BINARY)

install:
sudo cp bin/bitlash /usr/local/bin/
sudo cp $(BINARY) $(BINPATH)
2 changes: 1 addition & 1 deletion src/bitlash-api.c → src/bitlash-api.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ jmp_buf env;
//
// doCommand: main entry point to execute a bitlash command
//
numvar doCommand(char *cmd) {
numvar doCommand(const char *cmd) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why const? There are many users building buffers with commands to pass to Bitlash. Wouldn't this be a breaking change for them?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's the reverse. Adding const here means "doCommand will not change the string passed in". Any callers that have a non-const char* can still pass it in normally, since converting from char * to const char * is always allowed, just not the reverse.

The rationale for this change was that a literal string is also const, so a line like:

char *foo = "bar";

results in a compiler warning (at least with -Wall).

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That’s cool. I am wondering if this will break the earlier versions of avr-gcc.

Would you please collect a list of API-breaking changes and build settings changes in your contribution so we have all of them on the table at once for discussion? Thanks!

-br

On Feb 5, 2014, at 8:58 AM, Matthijs Kooijman [email protected] wrote:

In src/bitlash-api.cpp:

@@ -45,7 +45,7 @@
//
// doCommand: main entry point to execute a bitlash command
//
-numvar doCommand(char cmd) {
It's the reverse. Adding const here means "doCommand will not change the string passed in". Any callers that have a non-const char
can still pass it in normally, since converting from char * to const char * is always allowed, just not the reverse.

The rationale for this change was that a literal string is also const, so a line like:

char *foo = "bar";
results in a compiler warning (at least with -Wall).


Reply to this email directly or view it on GitHub.

return execscript(SCRIPT_RAM, (numvar) cmd, 0);
}

Expand Down
2 changes: 1 addition & 1 deletion src/bitlash-builtins.c → src/bitlash-builtins.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ const prog_char builtin_table[] PROGMEM = {



byte findbuiltin(char *name) {
byte findbuiltin(const char *name) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why const?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See reply above.

const prog_char *wordlist = builtin_table;

while (pgm_read_byte(wordlist)) {
Expand Down
2 changes: 1 addition & 1 deletion src/bitlash-cmdline.c → src/bitlash-cmdline.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ byte putlbuf(char c) {

void pointToError(void) {
if (fetchtype == SCRIPT_RAM) {
int i = (char *) fetchptr - lbuf;
int i = (const char *) fetchptr - lbuf;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why const?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See reply above.

if ((i < 0) || (i >= LBUFLEN)) return;
speol();
while (i-- >= 0) spb('-');
Expand Down
14 changes: 7 additions & 7 deletions src/bitlash-eeprom.c → src/bitlash-eeprom.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -73,15 +73,15 @@ int findend(int addr) {


// return true if string in EEPROM at addr matches string at str
char eestrmatch(int addr, char *str) {
char eestrmatch(int addr, const char *str) {
while (*str) if (eeread(addr++) != *str++) return 0;
if (eeread(addr) == 0) return 1; // ended at the same place?
return 0;
}


// find an entry in the db; return offset of id or FAIL
int findKey(char *id) {
int findKey(const char *id) {
int start = STARTDB;
while (start < ENDDB-4) {
// find the next entry
Expand All @@ -100,7 +100,7 @@ int start = STARTDB;


// Look up an entry by key. Returns -1 on fail else addr of value.
int getValue(char *key) {
int getValue(const char *key) {
int kaddr = findKey(key);
return (kaddr < 0) ? kaddr : findend(kaddr);
}
Expand Down Expand Up @@ -134,7 +134,7 @@ int starthole = STARTDB, endhole;
//

// Save string at str to EEPROM at addr
void saveString(int addr, char *str) {
void saveString(int addr, const char *str) {
while (*str) eewrite(addr++, *str++);
eewrite(addr, 0);
}
Expand All @@ -150,7 +150,7 @@ int erasestr(int addr) {
}

// erase entry by id
void eraseentry(char *id) {
void eraseentry(const char *id) {
int entry = findKey(id);
if (entry >= 0) erasestr(erasestr(entry));
}
Expand Down Expand Up @@ -180,10 +180,10 @@ char id[IDLEN+1]; // buffer for id
// fetchptr is on the character after '{'
//
// BUG: This is broken for file scripts
char *startmark = (char *) fetchptr; // mark first char of macro text
const char *startmark = (const char *) fetchptr; // mark first char of macro text
void skipstatement(void);
skipstatement(); // gobble it up without executing it
char *endmark = (char *) fetchptr; // and note the char past '}'
const char *endmark = (const char *) fetchptr; // and note the char past '}'

// endmark is past the closing '}' - back up and find it
do {
Expand Down
File renamed without changes.
19 changes: 8 additions & 11 deletions src/bitlash-functions.c → src/bitlash-functions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ numvar func_pulsein(void) { reqargs(3); return pulseIn(arg1, arg2, arg3); }
numvar func_snooze(void) { reqargs(1); snooze(arg1); return 0; }
numvar func_delay(void) { reqargs(1); delay(arg1); return 0; }

#if !defined(TINY_BUILD)
#if defined(SOFTWARE_SERIAL_TX)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don't remove the TINY_BUILD mechanism.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not removed, but setBaud is needed only if SOFTWARE_SERIAL_TX is enabled, which is not necessarily the same as TINY_BUILD being disabled. Doing this allows controlling SOFTWARE_SERIAL_TX separately from TINY_BUILD

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, thanks for the clarification. I can live with that.

The build flags are not as clean as I’d like but to a degree it cannot be helped.

-br

On Feb 5, 2014, at 9:01 AM, Matthijs Kooijman [email protected] wrote:

In src/bitlash-functions.cpp:

@@ -176,7 +176,7 @@ numvar func_constrain(void) {
numvar func_snooze(void) { reqargs(1); snooze(arg1); return 0; }
numvar func_delay(void) { reqargs(1); delay(arg1); return 0; }

-#if !defined(TINY_BUILD)
It is not removed, but setBaud is needed only if SOFTWARE_SERIAL_TX is enabled, which is not necessarily the same as TINY_BUILD being disabled. Doing this allows controlling SOFTWARE_SERIAL_TX separately from TINY_BUILD


Reply to this email directly or view it on GitHub.

numvar func_setBaud(void) { reqargs(2); setBaud(arg1, arg2); return 0; }
#endif

Expand All @@ -189,14 +189,14 @@ numvar func_bitread(void) { reqargs(2); return (arg1 & ((numvar)1 << arg2)) != 0
numvar func_bitwrite(void) { reqargs(3); return arg3 ? func_bitset() : func_bitclear(); }

numvar func_getkey(void) {
if (getarg(0) > 0) sp((char *) getarg(1));
if (getarg(0) > 0) sp((const char *) getarg(1));
while (!serialAvailable()) {;} // blocking!
return (numvar) serialRead();
}

numvar func_getnum(void) {
numvar num = 0;
if (getarg(0) > 0) sp((char *) getarg(1));
if (getarg(0) > 0) sp((const char *) getarg(1));
for (;;) {
while (!serialAvailable()) {;} // blocking!
int k = serialRead();
Expand Down Expand Up @@ -274,7 +274,9 @@ const prog_char functiondict[] PROGMEM = {
"abs\0"
"ar\0"
"aw\0"
#if defined(SOFTWARE_SERIAL_TX)
"baud\0"
#endif
"bc\0"
"beep\0"
"br\0"
Expand Down Expand Up @@ -351,7 +353,9 @@ const bitlash_function function_table[] PROGMEM = {
func_abs,
func_ar,
func_aw,
#if defined(SOFTWARE_SERIAL_TX)
func_setBaud,
#endif
func_bitclear,
func_beep,
func_bitread,
Expand Down Expand Up @@ -383,13 +387,6 @@ const bitlash_function function_table[] PROGMEM = {
};
#endif

// Enable USER_FUNCTIONS to include the add_bitlash_function() extension mechanism
// This costs about 256 bytes
//
#if !defined(TINY_BUILD)
#define USER_FUNCTIONS
#endif

#ifdef USER_FUNCTIONS
#define MAX_USER_FUNCTIONS 20 // increase this if needed, but keep free() > 200 ish
#define USER_FUNCTION_FLAG 0x80
Expand Down Expand Up @@ -441,7 +438,7 @@ void addBitlashFunction(const char *name, bitlash_function func_ptr) {
// find_user_function: find id in the user function table.
// return true if found, with the user function token in symval (with USER_FUNCTION_FLAG set)
//
char find_user_function(char *id) {
char find_user_function(const char *id) {
symval = 0;
while (symval < bf_install_count) {
if (!strcmp(id, user_functions[symval].name)) {
Expand Down
41 changes: 22 additions & 19 deletions src/bitlash-instream.c → src/bitlash-instream.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,19 +35,19 @@
***/
#include "bitlash.h"

#if defined(SDFILE)
#if defined(SDFILE) || defined(UNIX_BUILD)

#define O_READ 0x01 // from SdFile.h

// Trampolines for the SD library
byte scriptfileexists(char *scriptname);
byte scriptopen(char *scriptname, numvar position, byte flags);
byte scriptfileexists(const char *scriptname);
byte scriptopen(const char *scriptname, numvar position, byte flags);
numvar scriptgetpos(void);
byte scriptread(void);
byte scriptwrite(char *filename, char *contents, byte append);
byte scriptwrite(const char *filename, const char *contents, byte append);
void scriptwritebyte(byte b);
#elif !defined(UNIX_BUILD)
byte scriptfileexists(char *scriptname) { return 0; }
#else
byte scriptfileexists(const char *scriptname) { return 0; }
#endif

// masks for stashing the pointer type in the high nibble
Expand All @@ -60,7 +60,7 @@ byte scriptfileexists(char *scriptname) { return 0; }
#endif

// forward declaration
void initparsepoint(byte scripttype, numvar scriptaddress, char *scriptname);
void initparsepoint(byte scripttype, numvar scriptaddress, const char *scriptname);


/////////
Expand All @@ -72,7 +72,7 @@ void initparsepoint(byte scripttype, numvar scriptaddress, char *scriptname);
// and in runBackgroundTasks to kick off the background run.
//
//
numvar execscript(byte scripttype, numvar scriptaddress, char *scriptname) {
numvar execscript(byte scripttype, numvar scriptaddress, const char *scriptname) {

// save parse context
parsepoint fetchmark;
Expand Down Expand Up @@ -137,8 +137,8 @@ numvar execscript(byte scripttype, numvar scriptaddress, char *scriptname) {
// how to access the calling and called function names
//
//#define callername ((char *) ((numvar *) arg[2]) [1])
#define callername (arg[2] ? (char* ) (((numvar *) arg[2]) [1]) : NULL )
#define calleename ((char *) arg[1])
#define callername (arg[2] ? (const char* ) (((numvar *) arg[2]) [1]) : NULL )
#define calleename ((const char *) arg[1])


/////////
Expand Down Expand Up @@ -195,7 +195,7 @@ void markparsepoint(parsepoint *p) {
}


void initparsepoint(byte scripttype, numvar scriptaddress, char *scriptname) {
void initparsepoint(byte scripttype, numvar scriptaddress, const char *scriptname) {

#ifdef PARSER_TRACE
if (trace) {
Expand Down Expand Up @@ -234,12 +234,12 @@ void initparsepoint(byte scripttype, numvar scriptaddress, char *scriptname) {

#ifdef UNIX_BUILD

char *topname = ".top.";
const char *topname = ".top.";

void returntoparsepoint(parsepoint *p, byte returntoparent) {
// restore parse type and location; for script files, pass name from string pool
byte ftype = p->fetchtype;
char *scriptname = calleename;
const char *scriptname = calleename;
if (returntoparent) {
if ((ftype == SCRIPT_NONE) || (ftype == SCRIPT_RAM))
scriptname = topname;
Expand Down Expand Up @@ -297,7 +297,7 @@ void fetchc(void) {
//
void primec(void) {
switch (fetchtype) {
case SCRIPT_RAM: inchar = *(char *) fetchptr; break;
case SCRIPT_RAM: inchar = *(const char *) fetchptr; break;
case SCRIPT_PROGMEM: inchar = pgm_read_byte(fetchptr); break;
case SCRIPT_EEPROM: inchar = eeread((int) fetchptr); break;

Expand Down Expand Up @@ -327,7 +327,7 @@ void primec(void) {
void traceback(void) {
numvar *a = arg;
while (a) {
sp((char *) (a[1])); speol();
sp((const char *) (a[1])); speol();
a = (numvar *) (a[2]);
}
}
Expand All @@ -340,10 +340,10 @@ numvar *a = arg;
// "cat": copy file to serial out
//
numvar sdcat(void) {
if (!scriptfileexists((char *) getarg(1))) return 0;
if (!scriptfileexists((const char *) getarg(1))) return 0;
parsepoint fetchmark;
markparsepoint(&fetchmark);
initparsepoint(SCRIPT_FILE, 0L, (char *) getarg(1));
initparsepoint(SCRIPT_FILE, 0L, (const char *) getarg(1));
while (inchar) {
if (inchar == '\n') spb('\r');
spb(inchar);
Expand All @@ -358,7 +358,7 @@ numvar sdcat(void) {
//
// sdwrite: write or append a line to a file
//
numvar sdwrite(char *filename, char *contents, byte append) {
numvar sdwrite(const char *filename, const char *contents, byte append) {
#if !defined(UNIX_BUILD)
parsepoint fetchmark;
markparsepoint(&fetchmark);
Expand All @@ -373,6 +373,8 @@ numvar sdwrite(char *filename, char *contents, byte append) {
return 1;
}

// fprintf needs SERIAL_OVERRIDE to work
#ifdef SERIAL_OVERRIDE
//////////
//
// func_fprintf(): implementation of fprintf() function
Expand All @@ -382,7 +384,7 @@ numvar func_fprintf(void) {
parsepoint fetchmark;
markparsepoint(&fetchmark);

scriptwrite((char *) getarg(1), "", 1); // open the file for append (but append nothing)
scriptwrite((const char *) getarg(1), "", 1); // open the file for append (but append nothing)

//serialOutputFunc saved_handler = serial_override_handler; // save previous output handler
void scriptwritebyte(byte);
Expand All @@ -397,5 +399,6 @@ numvar func_fprintf(void) {
#endif
returntoparsepoint(&fetchmark, 1);
}
#endif

#endif // SDFILE
File renamed without changes.
Loading