search

teslaworksumn / HeadMaster

DMX parsing and I2C output for the master Animatronic Heads controller
MIT License
2 stars 0 forks source link

Make Project XC8 and CCI compliant #14

Closed taylortrimble closed 11 years ago

taylortrimble commented 11 years ago

This is a work in progress. I'd like to define some clean, consistent coding style here and also make our project use the super-sexy new (aka non-deprecated) XC8 compiler with CCI portability.

Use this Pull Request to discuss coding style as well.

We'll have to manually merge most of our code at one point or another because of this, but @aterlumen or I can help with that.

taylortrimble commented 11 years ago

@CrasherCourse @aterlumen EEW! I'm old and crusty and nobody's said anything yet! Am I ready to pull?

CrasherCourse commented 11 years ago

I looked through the code, didn't find any major errors, and it compiles just fine. I do get advisory 1233 and 1234 and warning 1273 when I compile in the old MPLAB. 1234 is a little scary since it's says corrupted data can occur (if async reset occurs).

A small note: The predefined values of slave address and the first register of the buffer were just for seeing if the debugger would let's us test I2C at the early stages of the code. We never got it to work back then. You can delete those values if you want.

Otherwise, I fairly confident that you can merge these files.

kevana commented 11 years ago

At band til 11, can't fix anything until then, can probably merge after the formatting stuff is fixed. On Nov 7, 2012 6:24 PM, "Taylor Trimble" notifications@github.com wrote:

@CrasherCourse https://github.com/CrasherCourse @aterlumenhttps://github.com/aterlumenEEW! I'm old and crusty and nobody's said anything yet! Am I ready to pull?

— Reply to this email directly or view it on GitHubhttps://github.com/teslaworksumn/HeadMaster/pull/14#issuecomment-10171290.

taylortrimble commented 11 years ago

@aterlumen Yeah I'll probably wait until #15 is in, fart with it some more, and then merge this in. @CrasherCourse Thanks for the thorough testing! Nice :clap: I'll get rid of the testing stuff in a future Pull Request, I'll keep this one laser focused on compiler though. Opening an issue preemptively.

taylortrimble commented 11 years ago

This is complex and will never be able to be auto-merged: FYI

taylortrimble commented 11 years ago

Okay, Pulling #15 left some build errors, which is okay! We'll fix those here as we transition to the new compiler, which may be lenient on the stuff that is causing #15 to fail. I'll add more throughout the day, please check it out so we can get this in ASAP.

Thanks!

taylortrimble commented 11 years ago

@CrasherCourse @aterlumen Hey guys! Review this please! I think it could be ready to pull soon :wink:

CrasherCourse commented 11 years ago

Tested the branch and it compiles for me.

taylortrimble commented 11 years ago

:grinning:

kevana commented 11 years ago

Go for it.

taylortrimble commented 11 years ago

Woot! Thanks for all the help with this guys!

John-Phillips commented 11 years ago

AAAAAAAAAAAAAAAhhhhhhhhhhh yea @aterlumen @CrasherCourse and @tylrtrmbl! You all are doing fantastic work. Keep on rocking!