Hi,
first of all thanks for the code. As i intend to build an RC Transmitter myself, this is exactly what i was looking for.
I tried the example code for PPM_out on an arduino Nano. As it did not work as expected, i looked at the library's PPMout.cpp code.
I came across the recomendation of pin 9 and 10 as output, as these can be configured to toggle automaticly on an output compare event. The problem i found is, pin 10 is OC1B in AVR-Terms. You configure it to toggle on an output compare event, but always use OCR1A to generate the event. If you wan't to use the toggle mode of pin 10, you will have to use OCR1B instead...
... or did i miss something ?
Furthermore the m_port member of PPMout-class is uninitialized, which might lead to an unwanted modification of some random port in the isr-subroutine when using pin 9 or 10.
Thanks again, and keep up the good work!
Sven
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
I thought I tested this stuff... Anyway, thanks for the bug report! I think you're right about the OCR1A/B, I'll test it again to verify its behavior. The datasheet isn't entirely clear about how it works exactly, but you're probably right.
The uninitialized member is just sloppy :(
I've created a ticket [#57] with the issues, it's on the list for the next release!
as all the data you're gonna store in those fields is uint16_t!
After fixing this, my PWM output is finally stable! Before it flickered an then crashed, i just wasn't sure if it was my code or yours. Your example might have worked because there were no globals after the definition of g_work, so nobody got hurt.
Sven
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Good grief, that's beyond sloppy... Nice job finding this! I've updated the ticket with this info, I hope I have some time this weekend to fix this and release a patch. It'll be fixed in v0.4 anyway since it no longer needs user supplied work buffers :)
Thanks again!
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Hi,
first of all thanks for the code. As i intend to build an RC Transmitter myself, this is exactly what i was looking for.
I tried the example code for PPM_out on an arduino Nano. As it did not work as expected, i looked at the library's PPMout.cpp code.
I came across the recomendation of pin 9 and 10 as output, as these can be configured to toggle automaticly on an output compare event. The problem i found is, pin 10 is OC1B in AVR-Terms. You configure it to toggle on an output compare event, but always use OCR1A to generate the event. If you wan't to use the toggle mode of pin 10, you will have to use OCR1B instead...
... or did i miss something ?
Furthermore the m_port member of PPMout-class is uninitialized, which might lead to an unwanted modification of some random port in the isr-subroutine when using pin 9 or 10.
Thanks again, and keep up the good work!
Sven
I thought I tested this stuff... Anyway, thanks for the bug report! I think you're right about the OCR1A/B, I'll test it again to verify its behavior. The datasheet isn't entirely clear about how it works exactly, but you're probably right.
The uninitialized member is just sloppy :(
I've created a ticket [#57] with the issues, it's on the list for the next release!
Related
Tickets: #57
Last edit: dvdouden 2013-03-13
Your welcome...
... may i add another fix then ? The computation of
define PPMOUT_WORK_SIZE(channels) ((channels) + (((channels) + 1) * 4))
should be
define PPMOUT_WORK_SIZE(channels) ((channels)*2 + (((channels) + 1) * 4))
or to make it more intuitive
define PPMOUT_WORK_SIZE(channels) (((channels) + ((channels) + 1) * 2) * sizeof(uint16_t))
as all the data you're gonna store in those fields is uint16_t!
After fixing this, my PWM output is finally stable! Before it flickered an then crashed, i just wasn't sure if it was my code or yours. Your example might have worked because there were no globals after the definition of g_work, so nobody got hurt.
Sven
Good grief, that's beyond sloppy... Nice job finding this! I've updated the ticket with this info, I hope I have some time this weekend to fix this and release a patch. It'll be fixed in v0.4 anyway since it no longer needs user supplied work buffers :)
Thanks again!