snu-quiqcl / qiwis

QuIqcl Widget Integration Software
MIT License
5 stars 2 forks source link

[FEATURE] Simplify broadcasting in apps #133

Closed BECATRUE closed 1 year ago

BECATRUE commented 1 year ago

Feature you want to implement

Currently the apps emits signals and connects slots to broadcast or to receive broadcast messages. This style is not so intuitive nor convenient.

Therefore, we want to provide an easy way to broadcast and receive messages.

How the feature is implemented

Basically BaseApp class will be modified.

1. Broadcasting

Implement a simple 'wrapper' method as follows:

def broadcast(self, channel: str, content: Any):
  content = json.dumps(content)
  self.broadcastRequested.emit(channel, content)

2. Receiving

Implement a pre-connected slot as follows:

def receivedSlot(self, channel: str, content: Any):
  """This will be overridden by child classes."""

@pyqtSlot(str, str)
def _receivedMessage(self, channel: str, msg: str):
  """This is connected to self.received signal."""
  self.receivedSlot(channel, json.loads(msg))

A child class which inherits BaseApp will then override receivedSlot() to handle the received messages. Note that the class can still implement its own slots for received signal. We just provide a new convenient way in addition to the previous method.

kangz12345 commented 1 year ago

I changed the label refactor -> enhancement because this provides a new feature which is similar to syntax sugars. When we say "refactoring", it usually does not change the code behavior (API).

BECATRUE commented 1 year ago

I just implemented the above three methods in BaseApp.

Before applying it into all apps, please confirm these methods. It can be checked in BECATRUE/133/simplify-handle-signals branch.

@kangz12345

kangz12345 commented 1 year ago

Oh, I found that we shouldn't check if the content is instance of str in broadcast(), because it is JSON-loaded when received anyway. I updated the example code above.

kangz12345 commented 1 year ago

In addition my reviews:

  1. In line 63, it should be pyqtSlot rather than pyqtSignal.
  2. In __init__(), you should connect _recevedMessage() to received signal.

@BECATRUE

BECATRUE commented 1 year ago

Oh, I found that we shouldn't check if the content is instance of str in broadcast(), because it is JSON-loaded when received anyway. I updated the example code above.

I didn't know at all, too😂