OpenBCI / OpenBCI_WIFI

Firmware library that runs on the OpenBCI WiFi Shield
https://shop.openbci.com/collections/frontpage/products/wifi-shield?variant=44534009550
MIT License
34 stars 30 forks source link

make and src/ #53

Closed jnaulty closed 6 years ago

jnaulty commented 6 years ago

First commit moves src files into a src/ directory

Second Commit removes all of repetitive .mk files in favor of a more generic Makefile, this also includes updating the README.md file on using the make-centric build/flash workflow.

jnaulty commented 6 years ago

@aj-ptw what do you think?

andrewjaykeller commented 6 years ago

Heck yea John, does moving source files to src break programming in the arduino ide?

jnaulty commented 6 years ago

Will have to test. I haven't used the arduino ide for programming the board.

jnaulty commented 6 years ago

:+1: it works on arduino ide v1.8.1 :smile_cat:

andrewjaykeller commented 6 years ago

I’m good to merge this!

jnaulty commented 6 years ago

I found this, which helps support src/ for holding openbci source files. https://github.com/arduino/Arduino/wiki/Arduino-IDE-1.5:-Library-specification#source-code

For backward compatibility with Arduino 1.0.x, the library author may opt to place source code into the root folder, instead of the folder called src. In this case the 1.0 library format is applied and the source code is searched from the library root folder and the utility folder, for example:

jnaulty commented 6 years ago

I'm running on a Fedora 25 machine. It would be nice to test this on Mac and/or Windows...

andrewjaykeller commented 6 years ago

Okay so testing and first thing i had to change was

#########################################
#            Arduino Lib                #
#########################################
ARDUINO_LIBS = $(HOME)/Arduino/libraries

to

#########################################
#            Arduino Lib                #
#########################################
ARDUINO_LIBS = $(HOME)/Documents/Arduino/libraries

And then once that route was fixed, i got error:

$ make SKETCH=examples/WifiShieldTCPUDP/WifiShieldTCPUDP.ino
Creating core archive
make: *** No rule to make target `/Users/andrewkeller/Documents/Arduino/libraries/OpenBCI_Wifi/OpenBCI_Wifi_Definitions.h', needed by
 `/tmp/mkESP/WifiShieldTCPUDP_huzzah/WifiShieldTCPUDP_.cpp.o'.  Stop.
make: *** Waiting for unfinished jobs....

Any ideas?

To resolve this I:

$ rm -rf /tmp/mkESP/WifiShieldTCPUDP_huzzah/

And then i had to comment out lines 13 and 14 on src/OpenBCI_Wifi.h to make:

// #define RAW_TO_JSON
// #define MQTT
jnaulty commented 6 years ago

Hey @aj-ptw

issue, that's because the current DEFAULT sketch is broke unless you uncomment this.

andrewjaykeller commented 6 years ago

arduino puts the arduino-related libs in different folders based on what OS you're running.

Totes agree here, can we make the bash script unique to the OS? Like if linux, do this, if macOS, do this?

jnaulty commented 6 years ago

ya, the linux/mac/windows configure part hasn't been implemented yet. i will change it to work on Mac first, since us linux users are used to workarounds :stuck_out_tongue: but ya, in the future we should have that.

jnaulty commented 6 years ago

typical workflow in a lot of projects that require compiling source code is

 ./configure
 make
 make install

where ./configure sets up system-based makefiles. autotools is a pretty heavy-ended solution, but might be nice. Otherwise, we could cook up a configure script that would detect current OS and set the make variables accordingly

andrewjaykeller commented 6 years ago

@jnaulty so i just tested this code and it's working as expected!!! Thanks John!!

I made a PR into your branch to add fixes to the readme.

where ./configure sets up system-based makefiles

That makes so much sense!

andrewjaykeller commented 6 years ago

i will change it to work on Mac first, since us linux users are used to workarounds 😛

Lol the life of linux users :<

jnaulty commented 6 years ago

ya, it's a decent amount of work to setup/configure autotools. And right now, I'm not sure it's worth the effort. If all I have to do is make ARDUINO_LIBS=$HOME/Arduino/libraries SKETCH=SKETCH=examples/WifiShieldTCPUDP/WifiShieldTCPUDP.ino that's no big deal. There is a crafty workaround I think we could come up with using default variables and the environment so I could create a file holding linux environment variables, and source that before executing make

#> source my_special_env
#> make
andrewjaykeller commented 6 years ago

Honesty it’s probably fine for now, the commenting out the

#define RAW_TO_JSON
#define MQTT

Would be sweet to figure out how to do that from command line, cause then we could automate the builds!