Bug 41752 - Fixes for italc2.py
Fixes for italc2.py
Status: CLOSED FIXED
Product: UCS@school
Classification: Unclassified
Component: iTALC
UCS@school 4.1 R2
Other Linux
: P5 normal (vote)
: UCS@school 4.1 R2 vXXX
Assigned To: Sönke Schwardt-Krummrich
Florian Best
: interim-2
: 37567 40316 42835 (view as bug list)
Depends on:
Blocks: 41758
  Show dependency treegraph
 
Reported: 2016-07-07 17:57 CEST by Sönke Schwardt-Krummrich
Modified: 2016-11-24 10:46 CET (History)
2 users (show)

See Also:
What kind of report is it?: Bug Report
What type of bug is this?: 5: Major Usability: Impairs usability in key scenarios
Who will be affected by this bug?: 3: Will affect average number of installed domains
How will those affected feel about the bug?: 4: A User would return the product
User Pain: 0.343
Enterprise Customer affected?:
School Customer affected?: Yes
ISV affected?:
Waiting Support:
Flags outvoted (downgraded) after PO Review:
Ticket number:
Bug group (optional):
Max CVSS v3 score:


Attachments
patch (1.87 KB, patch)
2016-10-24 17:08 CEST, Florian Best
Details | Diff
patch (7.17 KB, patch)
2016-11-03 19:08 CET, Florian Best
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sönke Schwardt-Krummrich univentionstaff 2016-07-07 17:57:52 CEST
While debugging the windows iTALC client, several small glitches in our italc2.py have been found.
Comment 1 Sönke Schwardt-Krummrich univentionstaff 2016-07-07 18:21:28 CEST
ucs-school-umc-computerroom (8.0.3-1):
r70883 | Bug #41752: improved log output
r70882 | Bug #41752: do not raise an exception if iTALC client connection has been lost but write an log entry
r70881 | Bug #41752: emit signal "connected" if iTALC connection is really established
r70880 | Bug #41752: allow overwriting of values even if value has not changed
r70879 | Bug #41752: use deep copy in LockableAttribute

ucs-school-umc-computerroom.yaml:
r70884 | Bug #41752: added yaml
Comment 2 Florian Best univentionstaff 2016-07-08 18:18:41 CEST
*** Bug 40316 has been marked as a duplicate of this bug. ***
Comment 3 Florian Best univentionstaff 2016-07-21 18:51:59 CEST
FYI: on my system self._core_ready is always False.
Comment 4 Sönke Schwardt-Krummrich univentionstaff 2016-08-12 16:56:27 CEST
The iTALC core connection is used on top of the iTALC VNC connection.
The core connection is set up *after* the VNC connection emits a state change 
"disconnected" → "connected".

Tests have shown that the core connection is not ready/usable right after setup.
That's why _core_ready is set to False. After the first usage of the core connection, two state changes are triggered: 
"connected" → "disconnected" → "connected".
Now the core connection is ready for use ==> _core_ready is set to True and the
"connected" signal is emitted.

self.connected() checks by default if _core_ready==True if not specified by 
argument to ignore this variable (used to send initial sendGetUserInformationRequest() via core connection to trigger connection state change).

ucs-school-umc-computerroom.yaml:
r71579 | Bug #41752: updated yaml
r70884 | Bug #41752: added yaml

ucs-school-umc-computerroom (8.0.4-1):
r71578 | Bug #41752: fixed connected logic
r71032 | Bug #41752: check boolean prior to function call
r70910 | Bug #41752: check if client is connected before requesting a screenshot
r70883 | Bug #41752: improved log output
r70882 | Bug #41752: do not raise an exception if iTALC client connection has been lost but write an log entry
r70881 | Bug #41752: emit signal "connected" if iTALC connection is really established
r70880 | Bug #41752: allow overwriting of values even if value has not changed
r70879 | Bug #41752: use deep copy in LockableAttribute
Comment 5 Sönke Schwardt-Krummrich univentionstaff 2016-08-16 16:36:06 CEST
The last commit was incomplete.

ucs-school-umc-computerroom (8.0.4-2):
r71645 | Bug #41752: fixed connected logic

8.0.4-2.186.201608161632
Comment 6 Florian Best univentionstaff 2016-08-16 17:11:15 CEST
The state detection is not reliable anymore. I can't (un)lock screens. I can start a presentation mode but in UMC there is no presentation stop button shown.
Comment 7 Sönke Schwardt-Krummrich univentionstaff 2016-10-21 14:10:44 CEST
(In reply to Florian Best from comment #6)
> The state detection is not reliable anymore. I can't (un)lock screens. I can
> start a presentation mode but in UMC there is no presentation stop button
> shown.

The hasChanged logic of the LockableAttributes was broken after introduction of the "force" kwarg in set(). Now, a change is recognized in set() and stored in a instance variable and no longer via comparison of old and current value.

Package: ucs-school-umc-computerroom
Version: 8.0.6-1.194.201610211407

r73448 | Bug #41752: fixed logic of "hasChanged"
Comment 8 Florian Best univentionstaff 2016-10-24 17:08:53 CEST
Created attachment 8148 [details]
patch

I am unsure about immediately resetting _has_changed when "asking" for hasChanged(). What do you think about an explicit reset after the update call? See Patch.
Comment 9 Sönke Schwardt-Krummrich univentionstaff 2016-11-03 11:13:00 CET
(In reply to Florian Best from comment #8)
> Created attachment 8148 [details]
> patch
> 
> I am unsure about immediately resetting _has_changed when "asking" for
> hasChanged(). What do you think about an explicit reset after the update
> call? See Patch.

I changed my mind since our last conversation. I think it's currently not neccessary. The more flexible way would be to connect to the signals emitted during state changes.
I we need this later on, we can still implement required changes.
Comment 10 Florian Best univentionstaff 2016-11-03 18:12:02 CET
The state of computers is not anymore detected. This leads to the problem that screen locking/input locking/etc. doesn't work:

Simple javascript snipped reveals the currently set values:
dojo.map(dijit.byId('umc_modules_computerroom_0')._grid.getAllItems(), function(item) { var item2 = {}; umc.tools.forIn(item, function(k, v) { item2[k] = v[0]; }); return item2; })
{
DemoClient:null
DemoServer:null
InputLock:null
MessageBox:null
ScreenLock:null
connection:"connected"
description:null
id:"WIN7-151"
ip:"10.200.27.151"
mac:""
name:"WIN7-151"
objectType:"computers/windows"
teacher:false
user:"jerman (Jerman Braend)"
}

The state for teachers is btw. correctly detected.
Comment 11 Florian Best univentionstaff 2016-11-03 18:20:14 CET
*** Bug 42835 has been marked as a duplicate of this bug. ***
Comment 12 Florian Best univentionstaff 2016-11-03 19:08:11 CET
Created attachment 8185 [details]
patch

self._core.slaveStateFlags() is initially always 0 therefore None was returned which is wrong. Attached patch removes this unnecessary check and removes duplicated logic in the frontend (since column.enablingMode == 'some' the canExecute function is already evaluated for the selection). This also fixes that the button is greyed out when the state is "None" (so that we don't need to see in the chromium console that no request was fired and are wondering why the screen locking doesn't work).
Comment 13 Sönke Schwardt-Krummrich univentionstaff 2016-11-04 15:14:48 CET
(In reply to Florian Best from comment #12)
self._core.slaveStateFlags() is initially 0 and switches then to 32 if (and only if!) the iTALC tray icon is enabled on the windows client. If the tray icon is disabled, the slaveStateFlags remain usually on 0 (if no other flag is set).

This might explain some problems seldom and sporadically reported by customers.
If the slaveStateFlags are 0, it is not possible to lock/unlock input/screens on iTALC clients.

Package: ucs-school-umc-computerroom
Version: 8.0.7-2.199.201611041442
Branch: ucs_4.1-0
Scope: ucs-school-4.1r2

ucs-school-umc-computerroom (8.0.7-2):
r74113 | Bug #41752: fixed logic for locking screen/input if iTALC icon has been disabled on windows client
Comment 14 Florian Best univentionstaff 2016-11-04 15:29:30 CET
OK: Fix
OK: YAML
Comment 15 Sönke Schwardt-Krummrich univentionstaff 2016-11-04 16:40:32 CET
Some javascript code has been merged. The grid buttons are now only enabled if the action is available at least for one computer. If state changes of computers are received from backend the grid buttons are updated automatically.

ucs-school-umc-computerroom (8.0.8-1):
r74115 | Bug #41752: several changes related to locking in computerroom

ucs-school-umc-computerroom.yaml:
r74116 | Bug #41752: updated advisory

Package: ucs-school-umc-computerroom
Version: 8.0.8-1.200.201611041632
Branch: ucs_4.1-0
Scope: ucs-school-4.1r2
Comment 16 Florian Best univentionstaff 2016-11-07 06:01:18 CET
OK: this works very nice now!
YAML: OK
Comment 17 Florian Best univentionstaff 2016-11-07 06:31:08 CET
*** Bug 37567 has been marked as a duplicate of this bug. ***
Comment 18 Sönke Schwardt-Krummrich univentionstaff 2016-11-10 16:00:43 CET
UCS@school 4.1 R2 v7 has been released.

http://docs.software-univention.de/changelog-ucsschool-4.1R2v7-de.html

If this error occurs again, please clone this bug.