Camera bar elements#29
Conversation
…ed Launch and Relaunch buttons, removed Add Camera button
|
wow those linter errors were actually very useful i'll review this shortly :) |
onkoe
left a comment
onkoe
left a comment
There was a problem hiding this comment.
looks good! just got a few clarity/alert nits to fix when convenient :D
| import CameraGrid, { type CameraContainer } from "./CameraGrid"; | ||
| import CameraToolbar from "./CameraToolbar"; | ||
|
|
||
| //filepath for testing (DELETE LATER): ../../../GitHub/Automomous/examples/ARTrackerTest/videos |
There was a problem hiding this comment.
might want to delete this before it gets re-discovered in five years
| //Need to create a selection of camera names to choose from, and then pass that camera name | ||
| //to the image source to get the video feed from the server |
There was a problem hiding this comment.
is this true? this kinda reads like a // TODO that's already been completed
| // IMPORTANT: Calls to the API should only run after this point. | ||
|
|
||
| /* API Requests Example | ||
| // Get the current mode (ex. 1920x1080 @ 30fps) | ||
| let modeResponse = await fetch(`/stream/cameras/${encodeURIComponent(selectedCameraPath)}/modes/current`); | ||
| console.log(await modeResponse.text()); | ||
|
|
||
| // Get the possible modes for the camera (ex. { 0: "1920x1080 @ 30fps", .. }) | ||
| let modesResponse = await fetch(`/stream/cameras/${encodeURIComponent(selectedCameraPath)}/modes`); | ||
| console.log(await modesResponse.json()); | ||
|
|
||
| // Set the current mode for the camera by the index found in the top api request | ||
| let setResponse = await fetch(`/stream/cameras/${encodeURIComponent(selectedCameraPath)}/modes/set/${1}`, { method: "PUT" }); | ||
| console.log(setResponse.status); | ||
| */ | ||
| }; |
There was a problem hiding this comment.
the peerConnection.onicecandidate lambda's scope ends here -- should these comments still be there?
| if (selectedCamera === "") { | ||
| alert("stream: no camera selected; cannot re-launch stream."); | ||
| return; | ||
| } |
There was a problem hiding this comment.
this could serve as a good sanity check. but you might just gray out the button instead
| const cameraContainer = cameraContainers.find(container => container.name === selectedCamera); | ||
|
|
||
| if (!cameraConnection) { | ||
| alert("stream: selected camera does not have an active connection; cannot re-launch stream."); |
There was a problem hiding this comment.
I think we should make any alert'd errors actionable just in case they're talking to ppl who are sweating in the desert 😭
| alert("stream: selected camera does not have an active connection; cannot re-launch stream."); | |
| alert(`stream: The selected camera is not plugged in | |
| (or is otherwise not responding), | |
| so the video streamer can't re-launch it. | |
| Please ensure it is plugged in correctly.`); |
| ); | ||
|
|
||
| if (!cameraContainer) { | ||
| alert("stream: selected camera does not have an available container; cannot re-launch stream."); |
There was a problem hiding this comment.
does this mean that we're out of hardcoded containers, or that the camera simply isn't assigned one? you might comment for clarity or replace the alert text
There was a problem hiding this comment.
we should probably remove this file
3 participants
UI Screenshot