OpenLightingProject
UART plugin recovers granularity if timing improves - closes #1798
Hi, with these changes the UART plugin can recover the granularity if timing improves, similar to how the FTDI plugin solves this issue.
I took care of the lint issues and the PR is now targeting the branch 0.10
Also your Git-Fu is clearly much better than mine getting it targeting 0.10 branch, well done!
I'm not sure if this was my error or yours, but there's one outstanding lint issue remaining if you wouldn't mind.
It should be fixed now.
Have you re-tested on the 0.10 branch?
Yes I did some testing on the 0.10 branch, and with these fixes, OLA behaved as expected.
I'm not sure if this was my error or yours, but there's one outstanding lint issue remaining if you wouldn't mind.
It should be fixed now.
Great thanks. For future reference you can also merge them directly from the web UI: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/incorporating-feedback-in-your-pull-request#applying-suggested-changes
Have you re-tested on the 0.10 branch?
Yes I did some testing on the 0.10 branch, and with these fixes, OLA behaved as expected.
That's brilliant news!
Just to be sure: The "write" returns immediately. So a working solution should probably get a timestamp at the moment the BREAK condition is released (A scheduling delay during the break then won't affect us), Then issue the write and calculate how long that should take, add in the mark-after-last-frame and wait until that moment before starting the next cycle.
When I was looking at the code, the person writing the code assumed that write would return when all bytes had been written. My suggestion above does not depend on whether the write returns immediately or not.
@peternewman Thanks for the info, I fixed the calculation of the frame time and it should honor, the boundaries now. I tested it as well, and the UART plugin still behaves as expected. Personally I don't like goto statements so I just split the code into more methods, but it still behaves the same (and it helped me with debugging).
@rewolff Thanks for your input, I incorporated this in my newest changes.
Glad to have been of help. I haven't been "intimate" with the code in a while, so good to hear that my "research" from a while back wasn't wasted and helped the actual code. :-)
I tweaked the code as you've suggested - thanks for the feedback! Hopefully, this PR can be included in 0.10.9. Unfortunately, it looks like I've somehow messed up the git history of my forked repo. I was able to push my code to the correct branch in the end, I just hope that this didn't mess up anything else.
I tweaked the code as you've suggested - thanks for the feedback! Hopefully, this PR can be included in 0.10.9.
🤞
Unfortunately, it looks like I've somehow messed up the git history of my forked repo. I was able to push my code to the correct branch in the end, I just hope that this didn't mess up anything else.
Yeah that's about what normally happens with me too!
Do you want to open a PR against this which I think is what you actually intend to merge now?: https://github.com/OpenLightingProject/ola/compare/0.10...markus983:ola:0.10
FWIW, although we've just cut 0.10.9, I'm planning to roll out 0.10.10 within the next few days ideally, or few weeks if a few key outstanding PRs like this one get resolved and merged.
Do you want to open a PR against this which I think is what you actually intend to merge now?: https://github.com/OpenLightingProject/ola/compare/0.10...markus983:ola:0.10
Yeah, that would make it easier. The "new" PR is now here: https://github.com/OpenLightingProject/ola/pull/1825