Socket issue #1115
Socket issue #1115
Conversation
| try { | ||
| const [devicesResponse, positionsResponse] = await Promise.all([fetch('/api/devices'), fetch('/api/positions')]); | ||
| if (devicesResponse.ok) { | ||
| dispatch(devicesActions.update(await devicesResponse.json())); | ||
| } | ||
| if (positionsResponse.ok) { | ||
| dispatch(sessionActions.updatePositions(await positionsResponse.json())); | ||
| } | ||
| connectSocket(); | ||
| } catch (error) { | ||
| // ignore errors | ||
| } |
There was a problem hiding this comment.
Why do we need to duplicate this?
There was a problem hiding this comment.
I'll remove the duplication
modern/src/SocketController.js
Outdated
|
|
||
| useEffect(() => { | ||
| if (authenticated) { | ||
| setTimeout(keepAlive, 3000); |
There was a problem hiding this comment.
Seems like this is called only once. Is that by design?
There was a problem hiding this comment.
Even if the value for the useEffect change?
useEffect(() => {
if (authenticated) {
setTimeout(keepAlive, 3000);
}
}, [lastUpdate]);
lastUpdate change every time a data in sent via the socket.onmessage, and the code inside the useEffect is re-executed
There was a problem hiding this comment.
Oh I see... but how do you cancel the old timeout?
There was a problem hiding this comment.
What about doing that?
const [timeOutId, setTimeOutId] = useState();
...
useEffect(() => {
if (authenticated) {
clearTimeout(timeOutId);
setTimeOutId(setTimeout(keepAlive, 3000));
}
}, [lastUpdate]);
There was a problem hiding this comment.
I guess it should work.
The only minor nit is use "timeout" as a single word, not "timeOut".
There was a problem hiding this comment.
I push the new changes, I'll give it a try on some devices and came back with the result
modern/src/SocketController.js
Outdated
| const resetCounterKeepAlive = () => { | ||
| setTimeKeepAlive(new Date()); | ||
| }; | ||
|
|
||
| const isConnected = () => Math.abs(new Date() - timeKeepAlive) < 10000; |
modern/src/SocketController.js
Outdated
| ContentProps={{ | ||
| sx: { | ||
| background: 'red', | ||
| }, | ||
| }} |
There was a problem hiding this comment.
Let's move out all unrelated changes like this.
modern/src/SocketController.js
Outdated
| const [events, setEvents] = useState([]); | ||
| const [notifications, setNotifications] = useState([]); | ||
|
|
||
| const [timeKeepAlive, setTimeKeepAlive] = useState(); |
There was a problem hiding this comment.
I would probably rename this to something like lastUpdate.
|
The main of the solution is it correct? I tried it on iOs and Android, didn't notice an issue on android, but on iOS the problem still persist of socket not updating |
2 participants
No description provided.