ola icon indicating copy to clipboard operation
ola copied to clipboard

Added '-padding' option to uartdmx plugin.

Open rbalykov opened this issue 4 years ago • 4 comments

Sets minimal slots count in UART frame. Default is 0, will behave as before.

rbalykov avatar Aug 02 '20 12:08 rbalykov

  1. FAIL: common/network/HealthCheckedConnectionTester - do I need to fix it some way?
  2. Missing from tarball plugins/uartdmx/README.md - i've checked it out from "master", edited and commited to "0.10". Need smth else?

rbalykov avatar Aug 04 '20 06:08 rbalykov

1. FAIL: common/network/HealthCheckedConnectionTester - do I need to fix it some way?

No, some of the tests area bit flaky on Mac, you can ignore this.

2. Missing from tarball plugins/uartdmx/README.md - i've checked it out from "master", edited and commited to "0.10". Need smth else?

On 0.10 the plugin description is in the code: https://github.com/OpenLightingProject/ola/blob/7892723801bc64c62dba0079ffe25d2f7a09c019/plugins/uartdmx/UartDmxPlugin.cpp#L114-L120

However I'd suggest if this code was to go in, that it should go in master, as it's a feature not a bug fix.

Can I ask the broader question, why? The web interface sends a full frame, any API can do so too, what's the reasoning for adding this to this plugin rather than dealing with it at source? I think the Enttec Pro's behave the same way and probably the Open's too. This would also be a perfect candidate for Patcher v2 #280.

peternewman avatar Aug 04 '20 15:08 peternewman

  1. Missing from tarball plugins/uartdmx/README.md - i've checked it out from "master", edited and commited to "0.10". Need smth else? On 0.10 the plugin description is in the code:

So, keep README or edit in-code descriptption? Plugin is still "Native UART", nothing has changed.

However I'd suggest if this code was to go in, that it should go in master, as it's a feature not a bug fix. Can I ask the broader question, why?

This code fixes #1523 , I got some "hardware madness" with zero-lenght frames. Sure, it could be fixed with ola_set_dmx, but this still gives some "broken" frames on boot. Had another PR #1573 with same issue before, but was unfamiliar with github, so mixed two PRs in one commit. This one gives just "-padding" relates code.

rbalykov avatar Aug 04 '20 15:08 rbalykov

Sorry this slipped through the net a bit.

  1. Missing from tarball plugins/uartdmx/README.md - i've checked it out from "master", edited and commited to "0.10". Need smth else? On 0.10 the plugin description is in the code:

So, keep README or edit in-code descriptption? Plugin is still "Native UART", nothing has changed.

Edit the in-code description if targetting 0.10 or the README if targetting master.

However I'd suggest if this code was to go in, that it should go in master, as it's a feature not a bug fix. Can I ask the broader question, why?

This code fixes #1523 , I got some "hardware madness" with zero-lenght frames.

Ah okay, makes sense.

Sure, it could be fixed with ola_set_dmx, but this still gives some "broken" frames on boot.

Yeah I think we're agreeing, fixing in the plugin is the most sense.

Had another PR #1573 with same issue before, but was unfamiliar with github, so mixed two PRs in one commit. This one gives just "-padding" relates code.

Great thanks.

peternewman avatar Jan 08 '21 02:01 peternewman