Closed joshuajnoble closed 11 years ago
Thanks for making these changes - they look good :)
I've added some comment to your diff. Can you please address them and submit an updated diff for me to consider?
What testing have you done of these changes?
I'll get those changes in, wasn't thinking before I hit PR. Testing can be done with the example applications in my repo.
Sorry I should clarify - I was wondering exactly what hardware you have been testing on?
I know you have been trying to get TX:Arduino, RX:ATTiny working but have you done any testing of TX:ATTiny, TX:Arduino to confirm that this has not been broken by your changes?
I've tested with Arduino Mega TX, ATTiny 85 RX and ATTiny 85 TX and ATTiny 85 RX. I haven't tested TX:Arduino Uno/etc, but those wouldn't be affected since different no interrupts are used for the sending and #ifdefs set up those interrupts. As far as the timing:
#define MinCount 15 //pulse lower count limit on capture
#define MaxCount 25 //pulse higher count limit on capture
#define MinLongCount 30 //pulse lower count on double pulse
#define MaxLongCount 50 //pulse higher count on double pulse
I simply worked them out by looking at the data coming from the oscilloscope and matched the interrupt timing and counts to that. Not the most mathematically rigorous approach, but it worked.
Thanks for the details about your testing - that looks great.
On 31 Oct 2012, at 00:02, joshua noble notifications@github.com wrote:
I've tested with Arduino Mega TX, ATTiny 85 RX and ATTiny 85 TX and ATTiny 85 RX. I haven't tested TX:Arduino Uno/etc, but those wouldn't be affected since different no interrupts are used for the sending and #ifdefs set up those interrupts. As far as the timing:
define MinCount 15 //pulse lower count limit on capture
define MaxCount 25 //pulse higher count limit on capture
define MinLongCount 30 //pulse lower count on double pulse
define MaxLongCount 50 //pulse higher count on double pulse
I simply worked them out by looking at the data coming from the oscilloscope and matched the interrupt timing and counts to that. Not the most mathematically rigorous approach, but it worked.
— Reply to this email directly or view it on GitHub.
I was thinking of this PR more as a "look at this" than something that should be merged in right now. I should have just put a link in this bug I filed https://github.com/mchr3k/arduino-libs-manchester/issues/2 to have you take a look at it. If you want to merge it in, that could be nice, but fwiw my feelings won't be hurt if you don't :)
I think it would be great to be able to merge your changes. It looks like you just need to do a little bit of tidy up and commenting.
I know your changes will be welcome as I have had several commenters on my blog wondering about how to get ATtiny RX working.
On 31 Oct 2012, at 00:24, joshua noble notifications@github.com wrote:
I was thinking of this PR more as a "look at this" than something that should be merged in right now. I should have just put a link in this bug I filed mchr3k/arduino-libs-manchester#2 to have you take a look at it. If you want to merge it in, that could be nice, but fwiw my feelings won't be hurt if you don't :)
— Reply to this email directly or view it on GitHub.
Handled better by https://github.com/mchr3k/arduino-libs-manchester/pull/4
This involved rewriting the interval timing and lengths for bit and double bit. Great work on the original library, this just adds a little bit of a extra functionality.