Adding Nodejs controller #240

izivkov · 2021-12-11T21:15:01Z

Adder Node.js based controller. Some minor fixes to main android app.

Some minor fixes to main android app.

…a row

…clients and easier to deal with.

…and reliability

…into Ivo/controller-js

izivkov · 2021-12-17T03:01:07Z

Sorry for the multiple commits. I was trying to squish all commits together.

thias15 · 2021-12-17T09:44:42Z

@izivkov, is this ready for review or are you still working on it?

thias15

Looks good, need to test.

If possible, it would be good to reduce the image size by saving as jpg.

I wonder if it makes sense to remove the RTSP connection in the controller app to reduce the apk size. Or do you see any benefit in using the RTSP connection over WebRTC?

thias15 · 2021-12-17T09:49:58Z

android/controller/src/main/java/org/openbot/controller/customComponents/Sound.kt


-    private fun onDataReceived(data: String) {
-        setOnOffStateConditions(data)
+//    private fun onDataReceived(data: String) {


Why is this function commented out? Since it is not used, is there value in keeping it in the code?

I moved the handling and the "mute" and "mirror" button to the client side. Before that, we would send a message to the server (main Android app), and the server will disable sound. This has some advantages, but it is cumbersome, and less reliable. For RTSP, initially I could not figure out how to mute the sound on the client, so I removed the speaker icon completely. I later figured it out, so this function is no longer needed. I have removed the commented code.

thias15 · 2021-12-17T09:54:51Z

controller-js/.eslintrc.json

@@ -0,0 +1,3 @@
+{


Is this file needed?

thias15 · 2021-12-17T09:59:17Z

controller-js/client/.eslintrc.json

@@ -0,0 +1,16 @@
+{


Is this file needed? Wouldn't it be automatically generated by users that install eslint. Or are there specific settings different from the standard settings?

thias15 · 2021-12-17T10:08:08Z

controller-js/server/.eslintrc.json

@@ -0,0 +1,17 @@
+{


See comments above.

izivkov · 2021-12-17T13:52:25Z

@izivkov, is this ready for review or are you still working on it?

It was supposed to be ready for review, but I keep thinking and adding enhancements. One last thing I wanted to add is a full-screen video mode. I had some issues in mirroring in full screen, but I will attempt to resolve today, and will let you know.

izivkov · 2021-12-17T14:34:40Z

Looks good, need to test.

If possible, it would be good to reduce the image size by saving as jpg.

I wonder if it makes sense to remove the RTSP connection in the controller app to reduce the apk size. Or do you see any benefit in using the RTSP connection over WebRTC?

About the JPG files, do you mean the images in the Readme file? Sure, I prob need to take another image again with the new buttons. I was using PNG because they are non-patented, and I thought would be preferred in a project like this.

We can remove RTSP from and android app, and this will reduce complexity, but we should also think if we need RTSP at all. Is is useful in the Python app. How about a separate PR for that?

izivkov · 2021-12-17T17:20:12Z

I made some of the changes you recommended. Could not get full-screen to work well, so let us go with what we have. After making sure everything works OK for you, we may add video resizing.
I think the PNG files do not add a lot of extra size. My screenshot tool generate PNG's, but anybody can further improve the Docs and the code.

izivkov · 2021-12-17T17:31:56Z

OK, I converted Screenshot.PNG to Screenshot.JPG, and it reduced the size somewhat.

izivkov · 2021-12-17T17:45:33Z

I have not tried this remotely, because my ISP has locked down my modem, and cannot open ports in the firewall. I would be curious if we can access the robot from outside the network.

thias15 · 2021-12-20T10:54:57Z

We can remove RTSP from and android app, and this will reduce complexity, but we should also think if we need RTSP at all. Is is useful in the Python app. How about a separate PR for that?separate PR for that?

Yes, as discussed separate PR. Let's remove RTSP from the controller app, but keep it for streaming via VLC or in the python app.

thias15 · 2021-12-20T11:27:18Z

Did some testing and it works very nicely. I'm getting a latency of 190ms. What did you measure? I have a few comments:

The video window changes size as the quality/resolution of the stream adapts to network quality. Would be nice to keep the window size fixed (maybe allow the user to select size, e.g. small, large, full-screen)
Pressing ESC disconnects from the server and I can no longer send controls. But the video stream continues. Also when changing the controller in the app, the video stream persists. This is not necessarily wrong but unexpected.
For me the default setting show the video stream 'mirrored`. So it would make sense to change the default/convention. How is it on your phone?

thias15 · 2021-12-20T11:34:12Z

Two more issue I encountered:

It seems the bot app cannot open the camera after WebRTC connection is established. For example, starting the stream in FreeRoam if I then go to Data Collection or Object Tracking I do not see the camera preview on the screen. WebRTC stream is still running though.
Toggling noise does not work. But this could be an issue in the robot app.

…iting full screen. Reset robot on shutdown.

Adding Nodejs controller

izivkov added 16 commits December 11, 2021 15:40

Adding Nodejs controller

55c2fa8

Some minor fixes to main android app.

Cleanup

edc94f0

Optimise sending drive commands by not sending same command twice in …

6ca2ae7

…a row

Update documentation

df97dbf

Small twicks

281f5e9

Fix documentation

8e8fa25

Added 'mute' button to UI

4de713d

Always set initial sound to 'muted'. This is more consistent for the …

7705475

…clients and easier to deal with.

Moved all muting and mirroring controls to the client for simplicity …

ad09485

…and reliability

Moved all muting and mirroring controls to the client for simplicity …

1827796

…and reliability

Merge branch 'Ivo/controller-js' of https://github.com/izivkov/OpenBot …

9e3aadb

…into Ivo/controller-js

Merge branch 'Ivo/controller-js' of https://github.com/izivkov/OpenBot …

efadf4f

…into Ivo/controller-js

Cleanup

d662336

Merge branch 'Ivo/controller-js' of https://github.com/izivkov/OpenBot …

34b855b

…into Ivo/controller-js

NodeJS controller

68fe153

Merge branch 'Ivo/controller-js' of https://github.com/izivkov/OpenBot …

d55732e

…into Ivo/controller-js

thias15 reviewed Dec 17, 2021

View reviewed changes

izivkov added 2 commits December 17, 2021 09:54

Cleanup after code review

cc9d449

Doc update

f273362

Converted some files gtom PNG to JPG

09996e6

izivkov added 3 commits December 17, 2021 15:42

Remove commented code

cb1bbfe

Removed unused localKeyboard. Cleanup code.

a38dea9

Minor cleanup

41fd6e2

Fix spelling

3982364

thias15 and others added 7 commits December 20, 2021 15:20

refactor controllers

7ba42c9

update readme

f02b37e

improve robot control

ce50361

Fix initial mirroing if image. Stop WrbRTC in Esc. Fix error after ex…

ead358e

…iting full screen. Reset robot on shutdown.

Added comments.

3c67d5b

Added button to go fullscreen

307229f

Fix spelling

3fd27df

thias15 approved these changes Dec 21, 2021

View reviewed changes

thias15 merged commit 31039cc into ob-f:master Dec 21, 2021

lacykaltgr pushed a commit to satinavrobotics/SatiBot that referenced this pull request Jun 1, 2025

Merge pull request ob-f#240 from izivkov/Ivo/controller-js

641a43b

Adding Nodejs controller

Adding Nodejs controller #240

Adding Nodejs controller #240

Uh oh!

Conversation

izivkov commented Dec 11, 2021

Uh oh!

izivkov commented Dec 17, 2021

Uh oh!

thias15 commented Dec 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thias15 left a comment

Choose a reason for hiding this comment

Uh oh!

thias15 Dec 17, 2021

Choose a reason for hiding this comment

Uh oh!

izivkov Dec 17, 2021

Choose a reason for hiding this comment

Uh oh!

thias15 Dec 17, 2021

Choose a reason for hiding this comment

Uh oh!

thias15 Dec 17, 2021

Choose a reason for hiding this comment

Uh oh!

thias15 Dec 17, 2021

Choose a reason for hiding this comment

Uh oh!

izivkov commented Dec 17, 2021

Uh oh!

izivkov commented Dec 17, 2021

Uh oh!

izivkov commented Dec 17, 2021

Uh oh!

izivkov commented Dec 17, 2021

Uh oh!

izivkov commented Dec 17, 2021

Uh oh!

thias15 commented Dec 20, 2021

Uh oh!

thias15 commented Dec 20, 2021

Uh oh!

thias15 commented Dec 20, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

thias15 commented Dec 17, 2021 •

edited

Loading