Skip to content

added ubc channel and broadcasting of dirty devices#79

Open
Jawou wants to merge 12 commits into
OpenLWR:dosefrom
Jawou:dose
Open

added ubc channel and broadcasting of dirty devices#79
Jawou wants to merge 12 commits into
OpenLWR:dosefrom
Jawou:dose

Conversation

@Jawou
Copy link
Copy Markdown
Collaborator

@Jawou Jawou commented May 3, 2026

i think sending the udp packet once then marking devices clean is not good idea unless the device is updating frequently enough that it doesnt matter

i think sending the udp packet once then marking devices clean is not good idea unless the device is updating frequently enough that it doesnt matter
Comment thread server/session_mgt.py Outdated

case rec_proto.RECMessageType.REC_REGISTER_SESSION:
client_ip = client.ClientAddress[0]
self.ubc_channel.register(client.SessionId, (client_ip, 1312))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the client won't be listening for UBC packets on 1312, but on an os- (or nat-)generated port, the registration flow will need to be changed, aiui it'll be as follows:

  1. client sends a keepalive packet to the server on ubc/uec (yes that would be the only c2s packet for ubc)
  2. server starts sending UBC/UEC packets to the new client host:port combo, including a session id
  3. client registers the session id over REC, linking the clients session to the client host:port combo for UBC/UEC

the exact details still need to be written in the spec, so this pr can't really be finished until then
@Stefan-5422 is the above correct?

Copy link
Copy Markdown

@Stefan-5422 Stefan-5422 May 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. The port 1312 is fine the port conversion/resolve is done by the operating system/nat. Correct, check the port addociated with the sessionid for correct port.

  2. The channel does only send a reply to the keepalive not any other packages until registered.

  3. Correct

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the client won't be listening for UBC packets on 1312, but on an os- (or nat-)generated port, the registration flow will need to be changed, aiui it'll be as follows:

1. client sends a keepalive packet to the server on ubc/uec (yes that would be the only c2s packet for ubc)

2. server starts sending UBC/UEC packets to the new client host:port combo, including a session id

3. client registers the session id over REC, linking the clients session to the client host:port combo for UBC/UEC

the exact details still need to be written in the spec, so this pr can't really be finished until then @Stefan-5422 is the above correct?

this is known, however i wanted this implemented now so i can continue working on the client

Comment thread server/communication/ubc_communication.py Outdated
@RedstoneParkour
Copy link
Copy Markdown
Contributor

perhaps another issue but not sure where to place it: UDP has a max packet size (the MTU), there's no protection against packets going above it
though autodetecting the MTU is quite complex (and might need another spec change)

i believe the rest is fine

Copy link
Copy Markdown

@Stefan-5422 Stefan-5422 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same concerns as @RedstoneParkour see comment replies. Also the transmission for this works under the principle that all data changes frequently, Maybe we should think of a sheme where a part (M%) of all devices gets sent along every N updates to avoid desyncs, i.e all clients get every device every 1/M*N updates.

@Stefan-5422
Copy link
Copy Markdown

Also I would be willing to merge thid before the protobug changes so that @watchful5406 can continue testing and implementing his 2D client

@RedstoneParkour
Copy link
Copy Markdown
Contributor

Maybe we should think of a sheme where a part (M%) of all devices gets sent along every N updates to avoid desyncs, i.e all clients get every device every 1/M*N updates.

Perhaps something like https://gafferongames.com/post/state_synchronization/#priority-accumulator would work? The packets for updated devices would have a higher priority (for the tick they got updated) than non-updated devices.

Though something like i'm suggesting should really be deferred for the future, maybe just dont test with too many devices at once :P

@Stefan-5422
Copy link
Copy Markdown

P

I meant ontop of dirty updates

@Jawou
Copy link
Copy Markdown
Collaborator Author

Jawou commented May 3, 2026

Same concerns as @RedstoneParkour see comment replies. Also the transmission for this works under the principle that all data changes frequently, Maybe we should think of a sheme where a part (M%) of all devices gets sent along every N updates to avoid desyncs, i.e all clients get every device every 1/M*N updates.

maybe add a property if a device has a high frequency change rate and a low frequency and update the low frequency change rate devices no matter thier dirty status every 1 second (60 ticks)

@Stefan-5422
Copy link
Copy Markdown

maybe add a property if a device has a high frequency change rate and a low frequency and update the low frequency change rate devices no matter thier dirty status every 1 second (60 ticks)

Also not a bad idea since refreshing high refresh rate items doesn't really do anything. Maybe this is settable per device type, since there probably arent any outliers there?

Comment thread server/example/simulation_module.py
@Stefan-5422
Copy link
Copy Markdown

What is this waiting on btw?

@RedstoneParkour
Copy link
Copy Markdown
Contributor

What is this waiting on btw?

I believe just https://gitlab.com/westfield-simulations/dose/spec/-/work_items/5

@Jawou
Copy link
Copy Markdown
Collaborator Author

Jawou commented May 9, 2026

What is this waiting on btw?

watchful told me to wait until the docs update

@watchful5406
Copy link
Copy Markdown
Member

with the update of the spec, we need to add heartbeat. i'll update the protobuf again

Comment thread server/communication/rec_communication.py
Comment thread server/protocols/common_pb2.py
Comment thread server/session_mgt.py Outdated
Comment thread server/session_mgt.py
Comment thread server/session_mgt.py Outdated
Comment thread server/communication/ubc_communication.py
Comment thread server/communication/ubc_communication.py Outdated
Jawou and others added 5 commits May 24, 2026 09:33
@RedstoneParkour
Copy link
Copy Markdown
Contributor

hmm another bug: if the client moves the switch to either CLOSE or TRIP and then back to neutral very quickly (for example because the client has it set up as a momentary switch) then the indicators don't update (but the flag does)
perhaps instead of checking the switch's position in the tick function the flag should be checked

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants