Yona-Appletree / LEDscape

Beagle Bone Black cape and firmware for driving a large number of WS281x LED strips.
126 stars 58 forks source link

channels are ignored by opcserver #15

Open colinfrei opened 9 years ago

colinfrei commented 9 years ago

It seems as if the channels passed in are ignored by the opcserver. I would expect it to map the channel to a pin on the board, but it doesn't seem to be doing that.

Yona-Appletree commented 9 years ago

This behavior is by design (see the README: https://github.com/Yona-Appletree/LEDscape#data-format). The rationale here is that LEDscape pushes out data to all pixels simultaneously, and if you wanted to push just one channel at a time, we'd either need some sort of syncing/flushing mechanism to know what to do with that data.

That being said, in the work of adding experimental support for e131, I did have to implement something similar to this (though it was a bit of a hack), and I may try to come up with a more general solution to allow this. One thought was the following:

Allow data on opc channels other than 0, such that channel N is mapped to N-1 and store those writes into a buffer that is written to and flushed by channel 0. You could then send as much channel data as you wanted, and then could flush it by sending an empty frame to channel 0. Would that be useful to you?

dbu commented 9 years ago

that sounds like a good idea to me. why would you map N to N-1?

channel 0 is supposed to write to all channels. if you consider the ports on the BBB to be channels, the current behaviour is not exactly to spec. we could do a mode of opc-server closer to the spec that uses 0 to broadcast to all channels. then the channels 1-48 could be mapped to the actual single outputs.

not sure how open http://openpixelcontrol.org/ is for adding own "commands" but it would imho make sense to define 2 new commands:

then people could choose for themselves if they have an issue with syncing and use buffer/flush, or want to set channel by channel separately and display immediately.

Yona-Appletree commented 9 years ago

I specified N-1 because we use zero-based channel indexes internally and on our capes. I'm just being explicit about opc using 1-based channels.

I'm torn on how to handle channel 0 / buffering / etc. Personally, I don't see a lot of use in the channel 0 broadcast mode (with my light art design hat on). Using it as a full buffer push is what fadecandy does and it's simple and clean for the frequent case where you want to push all data. One important consideration here is that ledscape must push data for all ports to the hardware at once (this is because the PRUs push out data to each port simultaneously). Any method for pushing single channels at a time that does not use a manual flush command will require some kind of automatic buffer flushing that will almost certainly result in (perhaps minor) visual artifacts.

I think it's important to consider why we want the opc-channels to map to ledscape ports. I can think of three reasons: API conformity, ease-of-use, and packet fragmentation. Lets look at each case specifically:

  1. API conformity is certainly a laudable thing to strive for and it enables easier interoperability with other systems. The easiest way to technically conform to the API would be to continue to treat our data buffer as a single channel, but follow the API and map it to opc channel
  2. Since channel 0 is push to all channels, our current implementation would continue to be accurate, but clients that conform to the spec could push to channel 1 if they preferred. Last I checked, fadecandy also accepts a full data buffer on channel 0, and I like having the drop-in compatibility with them.
  3. The ease-of-use / intuitive API argument is how I interpret @colinfrei's original comment for this ticket: We have multiple pins on the BBB, which "feel" like channels, why don't we map them to opc-channels? To which I will refer to my opening statement about how the hardware basically treats them as a single buffer. We certainly could implement this by having a thread that automatically flushes an internal buffer at a given frame rate, and figuring that if a user wants synchronization of buffer pushes, they can use a custom command. However, doing this would violate the opc spec unless we also made channel 0 push to all these new channels, which makes the common push-all-data-at-once use case harder (a custom opc command), which I don't want to do.
  4. The final argument is IP packet fragmentation. The maximum real-world payload size for a UDP packet without causing fragmentation is somewhere between 534 and 1472 bytes (http://stackoverflow.com/questions/14993000/the-most-reliable-and-efficient-udp-packet-size). This means that if you wish to avoid fragmentation, it would be ideal to be able to send single channel updates to opc-server. However, if you're concerned about ip-fragmentation (as at least one of our users is), you're probably advanced enough to also care about frame fragmentation, and so you want a custom sync signal as well. If we implemented this using the opc channels, it would break case 2, as require sync packet isn't all that ease-to-use, nor is it in the spec. So, for this case, the best way to handle this seems like using custom commands (which we already do, for configuration).

    From this, I make the following conclusions:

  5. We should at least make opc-server accept the full buffer on channel 1 as well as channel 0
  6. We should add custom packets for sending single channels and syncing
  7. I'm torn on adding more natural channel support because I don't want to get rid of a simple full-buffer update on channel 0 or 1

Thoughts?

David Buchmann mailto:notifications@github.com November 11, 2014 at 00:44

that sounds like a good idea to me. why would you map N to N-1?

channel 0 is supposed to write to all channels. if you consider the ports on the BBB to be channels, the current behaviour is not exactly to spec. we could do a mode of opc-server closer to the spec that uses 0 to broadcast to all channels. then the channels 1-48 could be mapped to the actual single outputs.

not sure how open http://openpixelcontrol.org/ is for adding own "commands" but it would imho make sense to define 2 new commands:

  • buffer data: send data for one channel without sending it out yet
  • flush data: send data to the channel. when received on channel 0, flushes all buffered data of all channels

then people could choose for themselves if they have an issue with syncing and use buffer/flush, or want to set channel by channel separately and display immediately.

— Reply to this email directly or view it on GitHub https://github.com/Yona-Appletree/LEDscape/issues/15#issuecomment-62517716.

Colin Frei mailto:notifications@github.com November 10, 2014 at 03:14

It seems as if the channels passed in are ignored by the opcserver. I would expect it to map the channel to a pin on the board, but it doesn't seem to be doing that.

— Reply to this email directly or view it on GitHub https://github.com/Yona-Appletree/LEDscape/issues/15.

Yona Appletree Senior Developer Concentric Sky, inc yona@concentricsky.com

dbu commented 9 years ago

Thanks a lot for taking the time to explain these things! It helps a lot to see the reasoning. I will see if i can put some of it into the README or a documentation file. So the ledscape_init num_pixels effectively defines the splitter after how many pixels the next pin should be used?

I think @colinfrei found that we can send the data on any opc channel currently and data is always distributed on all pins, regardless of the channel. So i think there is nothing to do for 1). stopping to accept data on channels > 1 sounds like an unnecessary BC break to me.

On the other side of fragmentation, we have another issue i think. We have a panel of 450 LEDs per pin. This seems to work well with the rgb-test program (once we got our cable connections right). We plan to have 2 BBB for a total of 72 panels, so 36 per BBB. If we would use all 48 pins, this would get us real close to the maximum UDP packet size of ~64k. Our current setup should land us at about 50k which should be ok. No idea if 36_1.5k would be better or worse than 1_50k but i guess we will see and come back on that topic if needed.

So all in all i think we can go with the current way things are handled and add more capabilities if it turns out we really need them :-)

Yona-Appletree commented 9 years ago

After having a conversation with the maker LED Lab it seems that there are UDP packet size limits causing issues for iOS users. As such, I propose the following modifications to LEDscape to fix this issue.

My proposed solution, to support your work (and other OPC clients), is the following:

This approach will allow:

Please let me know your thoughts. I'm open to critique and suggestions, especially on handling the auto synchronization thread.

schardt commented 9 years ago

Hi! I'm the aforementioned creator of LED Lab, which as of recently supports sending to Ledscape. http://appstore.com/ledlab I agree with your latest proposal, Yona (not the previous one(s))! My only suggestion would be: Instead of using an empty push to channel zero, why not send a manufacturer-dependent sysex code. Isn't what sysex is for?

colinfrei commented 9 years ago

If we're changing the behavior of the channel 0 push (and thus breaking BC), wouldn't it make sense to try to conform to the spec at the same time and use a command (either ask for it to be included in the spec or a system-specific command) instead?

schardt commented 9 years ago

BTW, I very much support the idea of Ledscape becoming a standard OPC controller! This would only only expand the number of clients that could use it, it would make LED Lab's UI simpler in that there would only need to be a single OPC driver (which would transmit the Ledscape-specific sync protocol too).

colinfrei commented 9 years ago

The immediate buffer flush is interesting though - we're currently having to work around the buffer to be able to display static images, and we do that by simply sending the same data multiple times. Being able to flush it directly sounds great!

Yona-Appletree commented 9 years ago

Adding a sysex command for manual syncing is probably a good idea, but I suggested the other mechanism for two reasons. First, I don't want to disallow pushing the full buffer on channel 0 as it's how fadecandy works (probably the next biggest OPC consumer) and also would break BC with LEDscape clients. Second, changing the zero-ing property is a minor change and actually brings us closer to the spec (which says pixels not included in a packet should be ignored).

So basically, to keep fadecandy and backwards compatibility, we have to support all-channels-on-0, which also does a sync. This gives us a sync for free without having to use custom commands and shouldn't cause issues with other spec-complient servers. That being said, it's more likely that another server will ignore a sysex command than it is to handle an empty channel 0 push correctly... so I think I agree that a sysex command is a good idea. But the channel 0 trick would work as well.

schardt commented 9 years ago

I agree with everything you've written (again)! Either trick would do.

But can we not have Ledscape handle a sysex for syncing AND also handle send-to-all-ouputs-on-channel-zero? We could then avoid the possibility of some OPC server barfing on a zero-length transmission to channel zero (although I'm not sure how likely this is). By definition, a server is required to harmlessly ignore sysexes sent to it that it doesn't understand.

Yona-Appletree commented 9 years ago

Sounds like we're on the same page. I see no reason not to do both (sysex and ch 0). I will implement this as time allows and update the documentation as well as closing this slip with any implementation notes I have.

schardt commented 9 years ago

Can you give me a clearer time as to when you can do this? There is at least one Ledscape/LED Lab user who's run up against a UDP packet size limit. How should we serve him right now? I can switch the RGB-123 driver over to using TCP for now. It's not that much work. But if you're going do this soon, then switching to TCP would be wasted work for me. And given the >=week that Apple takes to approve updates these days, wasted time too! :-)

Yona-Appletree commented 9 years ago

I'll be working on it this week and weekend, though we have a big event coming up early next week, so it's hard to promise on a timeline. I'd give it an 80% probability that I'll have it done by early next week.

Yona-Appletree commented 9 years ago

Alright, well this took longer than expected, as I spent a bunch of time on tracking down some flickering issues. In any case, I have a beta build available in the issue_15_channel_data branch.

I wrote a small test client in examples/node/send-test-data.js that can be used as an example. You can invoke it in three ways:

node send-test-data.js --single true --sync true # Sends per-channel data with a sync frame
node send-test-data.js --single true --sync false # Sends per-channel data without a sync frame (auto syncing thread in opc-server is used)
node send-test-data.js --single false # Sends data in the old way, all data on channel 0

At the moment, the test client syncs using an empty push on channel 0, but there is also a system-specific command: Sysid=2, Cmdid=2. Where the Cmdid is the first byte of the system-specific payload.

Let me know how it works for you, I'd love to hear feedback.

schardt commented 9 years ago

Yona,

At the moment, the test client syncs using an empty push on channel 0, but there is also a system-specific command: Sysid=2, Cmdid=2. Where the Cmdid is the first byte of the system-specific payload.

How does LEDscape know that it should wait to update the LEDs, that a sync signal is coming?

Also, did you implement the usage of the output index? ie: Each output's data arrives in a separate packet?

It would be great if you could write a complete description of the changes you've made. I know we discussed this earlier, but it seems (with the sysex command, eg), that you've come up with some other ideas too.

Thanks! Christopher

Yona-Appletree commented 9 years ago

Chris,

I have implemented what was previously specified as closely as possible. The sysex command is optional, but a slightly more "correct" implementation.

To recap, it works like this:

To use it you simply start sending packets for channels > 0, and if you so choose, sending sync packets as empty payloads for ch 0 or via sysex command. That's it, really.

schardt commented 9 years ago

Yona,

Thanks for your time! I understand you've got a bit project.

To use it you simply start sending packets for channels > 0, and if you so choose, sending sync packets as empty payloads for ch 0 or via sysex command. That's it, really.

So to send data to output zero, I now send data to channel 1, and so on up to 48, right?

What will a stock OpenPixel controller do with all this? I imagine it will ignore the sysex. What will it do with a zero-length write to channel zero?

What I'd like to do is just have a single OpenPixel driver and send data in this new way to it. Do you see any problem in this?

Thanks again! Christopher

mykolasmith commented 9 years ago

Thanks, Yona. This is very useful. Any chance on merging this into the master?

kersten commented 9 years ago

have u got some updates on this?