New web UI, store highest channel number used #1739

cmouttet · 2021-08-01T15:37:26Z

With this fix the Fader UI and Keypad UI are able to send zeros to channels with the highest number.

Due to an optimisation the tailing zeros are cut off. With this stored highest channel number, we can fill up the DMX array with zeros until the maximum channel used.

peternewman

Great, thanks very much for this @cmouttet .

Given https://groups.google.com/g/open-lighting/c/IvFInOVAbTE do you want to maybe add a README.developer in olad/www maybe, signposting people to the other folders?

olad/www/new/css/style.min.css

javascript/new-src/src/factories/ola.js

peternewman · 2021-08-04T03:08:29Z

javascript/new-src/src/factories/ola.js

@@ -118,6 +134,7 @@ ola.factory('$ola', ['$http', '$window', 'OLA',
            }
          })
            .then(function(response) {
+              updatehighestChannelNumberUsed(response.data.dmx);


I forget the exact workflow here, but all updates to the faders come via this function do they? So both local and remote stuff is covered? Ideally this would just be called to deal with the local fader/keypad stuff to let OLAD deal with LTP merges.

I saw that the faders' data is updated by polling get_dmx. Remote updates are handled by the backend code. I think get_dmx will return the correctly updated/merged content.

Ah yeah you should be fine then, I just wasn't sure if there were a few different paths on the JS side of things. The backend will indeed deal with merging a remote update.

Although that might mean that bringing up a high numbered channel in e.g. the ola_dmxconsole will ripple into the JS faders too, where it perhaps didn't before, but I can't think of an easy solution to that.

javascript/new-src/src/factories/ola.js

…in dmxConvert

peternewman

A few minor tweaks and suggestions.

Have you checked if it fixes this:

Keypad on the beta UI supports setting a concrete channel to a value. If you set the last non-zero channel to zero, the current value doesn't change.

Originally posted by @cmouttet in #1280 (comment)

Also does blackout work as intended?

README.developer

AUTHORS

javascript/new-src/src/factories/ola.js

peternewman · 2021-08-06T22:32:47Z

javascript/new-src/src/factories/ola.js

+           channel--) {
+
+        if ((dmx[channel - 1] > OLA.MIN_CHANNEL_VALUE) &&
+            (highestChannelNumberUsed < channel)) {


If it did say (which as below I don't think it needs to, swapping the two variables around so it matches the for loop would be less confusing).

When using max only the > OLA.MIN_CHANNEL_VALUE part is needed. I removed the part highestChannelNumberUsed < channel.

peternewman · 2021-08-06T22:33:18Z

javascript/new-src/src/factories/ola.js

+    var highestChannelNumberUsed = 0;
+
+    var updateHighestChannelNumberUsed = function(dmx) {
+      for (var channel = dmx.length; channel > highestChannelNumberUsed;


A comment might be nice here, just so we don't automatically assume it's iterating from dmx.length to zero as I did. Something like "we only care about checking channels higher than our existing highest" or similar maybe.

peternewman · 2021-08-06T22:40:56Z

javascript/new-src/src/controllers/universe.js

@@ -24,6 +24,7 @@ ola.controller('universeCtrl',
      'use strict';
      $scope.dmx = [];
      $scope.Universe = $routeParams.id;
+      $ola.resetHighestChannelNumberUsed();


We're better off calling a function here than just initialising a scope variable then are we (like the DMX array)?

I'm not very familiar with JavaScript. My home is Java on the backend. :)
I tried to to work with something like this $scope.highestChannelNumberUsed = 0 but I failed.

So it seems like $scope should work if you change it everywhere, but only within one controller, or there's $rootScope, or our $ola might be able to take a variable.

Otherwise @daveol or @FloEdelmann might be able to help, all this whizzy JavaScript library stuff isn't really my area of expertise either unfortunately.

javascript/new-src/src/factories/ola.js

peternewman · 2021-08-06T22:44:52Z

javascript/new-src/src/factories/ola.js

+          if (highestChannelNumberUsed < channel) {
+            highestChannelNumberUsed = channel;
+          }


Again would $window.Math.max be easier?

Yes, it works. Thanks.

Co-authored-by: Peter Newman <peternewman@users.noreply.github.com>

peternewman

Sorry for the very delayed review @cmouttet . A few more minor comments if you've got some time? It would definitely be good to get this bugfix into 0.10.9.

peternewman · 2022-11-13T15:09:29Z

README.developer

+The sources for the new UI is located in javascript/new-src. The built artifacts
+is copied to olad/www/new/js/app.min.js.


Fix some grammar:

Suggested change

The sources for the new UI is located in javascript/new-src. The built artifacts

is copied to olad/www/new/js/app.min.js.

The sources for the new UI are located in javascript/new-src. The built artifacts

are copied to olad/www/new/js/app.min.js.

peternewman · 2022-11-13T15:10:29Z

AUTHORS

  Stefan Krüger, added APA102 support to the SPI Plugin
  Tobi Schäfer, for the MacPort files
  Stefan S, improved timing with monotonic clock


Sort the file:

Suggested change

Stefan Krüger, added APA102 support to the SPI Plugin

Tobi Schäfer, for the MacPort files

Stefan S, improved timing with monotonic clock

Stefan Krüger, added APA102 support to the SPI Plugin

Stefan S, improved timing with monotonic clock

Tobi Schäfer, for the MacPort files

peternewman · 2022-11-13T15:10:53Z

AUTHORS

@@ -29,3 +29,4 @@ Contributors:
  Stefan Krüger, added APA102 support to the SPI Plugin
  Tobi Schäfer, for the MacPort files
  Stefan S, improved timing with monotonic clock
+  Christian Mouttet, various fixes


Can you sort your name entry too please (which avoids any issues with who did the most).

Feel free to change the top line to be "Contributors (in alphabetical order):" or similar.

peternewman · 2022-11-13T15:15:26Z

javascript/new-src/src/factories/ola.js

+    var highestChannelNumberUsed = 0;
+
+    // Search for the highest channel in the array `dmx`
+    // that having a value greater than MIN_CHANNEL_VALUE


Grammar wise we want either:

Suggested change

// that having a value greater than MIN_CHANNEL_VALUE

// that has a value greater than MIN_CHANNEL_VALUE

Or:

Suggested change

// that having a value greater than MIN_CHANNEL_VALUE

// having a value greater than MIN_CHANNEL_VALUE

peternewman · 2022-11-13T15:16:08Z

javascript/new-src/src/factories/ola.js

+    // holds the highest channel that was used by faders or the keypad
+    var highestChannelNumberUsed = 0;
+
+    // Search for the highest channel in the array `dmx`


Would this help clarify your last sentence?

Suggested change

// Search for the highest channel in the array `dmx`

// Search for the new highest channel in the array `dmx`

peternewman · 2022-11-13T15:27:00Z

javascript/new-src/src/factories/ola.js

+      for (var channel = dmx.length; channel > highestChannelNumberUsed;
+           channel--) {
+
+        var value = parseInt(dmx[channel - 1], 10);


This should probably call channelValueCheck to sanity check the value the field was set to. But then updateHighestChannelNumberUsed is just dmxConvert without the return, so perhaps we should just have one function?

peternewman · 2022-11-13T15:34:42Z

javascript/new-src/src/controllers/universe.js

@@ -24,6 +24,7 @@ ola.controller('universeCtrl',
      'use strict';
      $scope.dmx = [];
      $scope.Universe = $routeParams.id;
+      $ola.resetHighestChannelNumberUsed();


So it seems like $scope should work if you change it everywhere, but only within one controller, or there's $rootScope, or our $ola might be able to take a variable.

Otherwise @daveol or @FloEdelmann might be able to help, all this whizzy JavaScript library stuff isn't really my area of expertise either unfortunately.

Christian Mouttet added 2 commits August 1, 2021 17:28

store highest channel number used. Fixes OpenLightingProject#1280

34a7769

add results from 'grunt build'

6d7a9af

peternewman linked an issue Aug 4, 2021 that may be closed by this pull request

The beta UI fader window, set all channels to zero doesn't work #1280

Open

peternewman added this to the 0.10.9 milestone Aug 4, 2021

peternewman requested changes Aug 4, 2021

View reviewed changes

Christian Mouttet added 3 commits August 4, 2021 21:25

add documentation

f5faaec

add resetHighestChannelNumberUsed on universe change, simplify logic …

da36879

…in dmxConvert

add results from 'grunt build'

8f443cb

peternewman requested changes Aug 6, 2021

View reviewed changes

peternewman changed the title ~~store highest channel number used~~ New web UI, store highest channel number used Aug 6, 2021

Update README.developer, javascript/new-src/src/factories/ola.js

4eacc64

Co-authored-by: Peter Newman <peternewman@users.noreply.github.com>

cmouttet force-pushed the 1280-fader-issues branch from 74061f5 to 4eacc64 Compare August 11, 2021 15:28

Christian Mouttet added 2 commits August 11, 2021 19:23

updates based on review annotations

1f57145

add results from 'grunt build'

c5b8700

peternewman requested changes Nov 13, 2022

View reviewed changes

kripton mentioned this pull request Feb 6, 2023

"New UI" tabs not working on Linux? #1782

Closed

peternewman modified the milestones: 0.10.9, 0.10.10 Feb 26, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New web UI, store highest channel number used #1739

New web UI, store highest channel number used #1739

cmouttet commented Aug 1, 2021

peternewman left a comment

peternewman Aug 4, 2021

cmouttet Aug 4, 2021

peternewman Aug 6, 2021

peternewman left a comment

peternewman Aug 6, 2021

cmouttet Aug 11, 2021 •

edited

peternewman Aug 6, 2021

peternewman Aug 6, 2021

cmouttet Aug 11, 2021

peternewman Nov 13, 2022

peternewman Aug 6, 2021

cmouttet Aug 11, 2021

peternewman left a comment

peternewman Nov 13, 2022

peternewman Nov 13, 2022

peternewman Nov 13, 2022

peternewman Nov 13, 2022

peternewman Nov 13, 2022

peternewman Nov 13, 2022

peternewman Nov 13, 2022

		The sources for the new UI is located in javascript/new-src. The built artifacts
		is copied to olad/www/new/js/app.min.js.

	// that having a value greater than MIN_CHANNEL_VALUE
	// that has a value greater than MIN_CHANNEL_VALUE

	// that having a value greater than MIN_CHANNEL_VALUE
	// having a value greater than MIN_CHANNEL_VALUE

	// Search for the highest channel in the array `dmx`
	// Search for the new highest channel in the array `dmx`

New web UI, store highest channel number used #1739

Are you sure you want to change the base?

New web UI, store highest channel number used #1739

Conversation

cmouttet commented Aug 1, 2021

peternewman left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peternewman left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cmouttet Aug 11, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peternewman left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cmouttet Aug 11, 2021 •

edited