search

adafruit / Adafruit_nRF8001

Drivers for Adafruit's nRF8001 Bluetooth Low Energy Breakout
109 stars 53 forks source link

Adding support for the ATmega256RFR2 #2

Closed quartzjer closed 10 years ago

quartzjer commented 10 years ago

This also fixes a complaint from the newer avr toolchain.

ladyada commented 10 years ago

Please verify you have tested this with an Arduino UNO - that it doesn't break backwards-compatibility. this will help us pull & merge faster!!!

quartzjer commented 10 years ago

Hmm, I don't have one, maybe @erictj does, if not I'll try to pick an UNO up soon :)

amcjen commented 10 years ago

I only have a Diecimlia, Duemilanove, and a Mega. :/

ladyada commented 10 years ago

test with duemilanove

microbuilder commented 10 years ago

I'm OK with adding this, but can you perhaps make some changes to the extra sample sketch to clarify what the target processor is, and explain exactly what this sketch is trying to do (show how to use the library with a different target than the Uno)? There are quite a few comments that might be confusing to beginners like the Uno pinout notes and then NOT using those pins, but no explanation why. The description in the comment header should probably be adjust to clarify everything as well.

quartzjer commented 10 years ago

@microbuilder Hmmm... I'm not sure what you mean? There's no changes to the existing sketch or any sketch added, this only let's the library code be used to target the 256rfr2?

It's just a fix for the compiler warning (Adafruit_BLE_UART.cpp) and adding to the list of all the targets (hal_aci_tl.cpp)?

microbuilder commented 10 years ago

I mean for example this:

+// Connect CLK/MISO/MOSI to hardware SPI +// e.g. On UNO & compatible: CLK = 13, MISO = 12, MOSI = 11 +#define ADAFRUITBLE_REQ SS +#define ADAFRUITBLE_RDY 4 // This should be an interrupt pin, on Uno thats #2 or #3 +#define ADAFRUITBLE_RST 8

And this:

+/***** +This is an example for our nRF8001 Bluetooth Low Energy Breakout

Since this sketch will NOT run on an Uno out of the box, it's misleading. Perhaps it's better to just remove the sample sketch and stick to the original two since I'm afraid this is going to be a headache for the support team?

microbuilder commented 10 years ago

As it is today, if I accept this pull request it also pulls in the new sample sketch. If you could separate them, that might make it easier to handle the two issues differently, since the third sketch is likely to cause confusion.

quartzjer commented 10 years ago

Hah that would be bad, the sketch shouldn't be changed! :)

Here's what I see in the diff on this pull req, what am I missing? http://jeremie.com/i/5b63d24079.png

microbuilder commented 10 years ago

Ah, sorry ... my mistake. I was looking at your repo, and it seems the pull request doesn't include that sample sketch. My apologies. I'm happy to merge this, then.

amcjen commented 10 years ago

Just missed the pull request merge, but tested on a Duemilanove, all looks good.

screen shot 2014-04-28 at 4 10 50 pm

slack_for_ios_upload_360

quartzjer commented 10 years ago

Ahh yes the fork had test code in it but the pull was from a clean branch, sorry for the confusion, and thanks for the awesome code+docs+module!

On Apr 28, 2014, at 5:00 PM, Kevin Townsend notifications@github.com wrote:

Ah, sorry ... my mistake. I was looking at your repo, and it seems the pull request doesn't include that sample sketch. My apologies. I'm happy to merge this, then.

— Reply to this email directly or view it on GitHub.