Conversation
1egoman
left a comment
1egoman
left a comment
There was a problem hiding this comment.
Generally makes sense to me, I'm not a flutter expert though so it would be good to also get a review from somebody who is.
| SessionOptions({ | ||
| Room? room, | ||
| this.preConnectAudio = true, | ||
| this.agentConnectTimeout = const Duration(seconds: 20), | ||
| }) : room = room ?? Room(); | ||
| this.encryption, | ||
| }) : isRoomProvided = room != null, | ||
| room = room ?? Room() { | ||
| _validateEncryptionConfiguration(); | ||
| } | ||
|
|
||
| SessionOptions._({ | ||
| required this.room, | ||
| required this.isRoomProvided, | ||
| required this.preConnectAudio, | ||
| required this.agentConnectTimeout, | ||
| this.encryption, | ||
| }) { | ||
| _validateEncryptionConfiguration(); | ||
| } |
There was a problem hiding this comment.
This might be a bad / nonsensical suggestion as I am not too familiar with typical flutter patterns, but would be worthwhile to get all session initialization (constructor and alternate constructor paths like the copyWith static method) going through the same core _ static method for initialization?
xianshijing-lk
left a comment
xianshijing-lk
left a comment
There was a problem hiding this comment.
one question and look good to me
| // Restart: the E2EE manager already exists from a previous session. | ||
| // Re-enable in case it was toggled off, before connect publishes | ||
| // any tracks. | ||
| await room.setE2EEEnabled(true); |
There was a problem hiding this comment.
curiously, do we need to pass the encryption over to the room in this case ?
|
Thanks for the review 🙏 final session = Session.withAgent(
'my-agent',
tokenSource: TokenSource.endpoint('https://my-api/token'),
options: SessionOptions(
encryption: await E2EEOptions.sharedKey('my-secret'),
),
);The await on E2EEOptions.sharedKey isn't great, and I was trying to move it to E2EEManager.setup(), but I think that's better left for another PR. |
4 participants
No description provided.