-
Notifications
You must be signed in to change notification settings - Fork 13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
implement htp and ltp merging receivers #58
Conversation
a6ccad0
to
3e1601b
Compare
Thanks for the PR! According to §6.2.3.4 of the E1.31 spec:
So we need to be very clear about how the algorithms work. This is my understanding of your code:
|
Your table looks good to me. HTPMerging is processed per universe in any case. The current implementation supports priority per source only.
LTPMerging is processed per universe in any case.
Interesting would be to
In case you're wondering why: |
src/receiver/ltp.ts
Outdated
mergedData.data[ch] = referenceData.data[ch] || 0; | ||
|
||
for (const [, tmp] of data.universeData.servers) { | ||
if (tmp.lastUpdate > referenceTime) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From #58 (comment):
LTP
[...]
- In case of different priorities:
The sender using the highest priority wins for the whole universe.
That sounds logical to me. However, the LTP algorithm doesn't consider the priority
right now - I think we need something like this:
if (tmp.lastUpdate > referenceTime) { | |
if (tmp.lastUpdate > referenceTime && tmp.priority === data.maximumPriority) { |
Also, it seems like the LTP algorithm has no way to reset a channel value to 0
? Once a channel is set to a non-0 value, it is impossible to set that channel back to 0
. I'm interested to know when this behaviour is useful?
I'm currently overhauling the implementation to support the difference between |
okay, I'm not sure if that's possible, because at the lowest level, channel values are encoded within a packet using 512 octets, each one can have a value between There are some options:
But even with these 2 mechanisms, I don't think you can achieve the behaviour that you're after. So LTP can only work per-universe, not per-channel. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've rebased this PR and made some changes to the logic. LTP is now per-universe, since it's impossible to implement LTP per-channel.
I'm going to merge this but keep it undocumented for now, until I have time to add more test cases.
Thanks again for the PR :)
This is a small extension of @hansSchall PR. It implements LTP and HTP-merging additionally.