adelyte / crescendo-for-crestron

Crescendo Framework for Crestron control systems.
https://www.adelyte.com/crestron/crescendo
Other
77 stars 26 forks source link

Wiredness with source lists on MC4 #100

Open sphere27 opened 4 years ago

sphere27 commented 4 years ago

I'm in the middle of a delivery on my 1st MC4. I'm noticing a few intermittent issues regarding the source lists that are being quite pesky to pin down.

One scenario - when I select a source for a room, the source that is calculated in the Rooms Source module ends up having the value '0' instead of the correct source ID. This causes a differnet source to be selected.

Second scenario - the source list doens't populate with the correct values. I have been unable to debug when this issue is present.

I believe that I'm working with a Crescendo suite from about a year ago. None of the recent notes seems to imply that stuff was changed in the room sources modules. But Ill get a new one down now.

Thanks for building this. Gotta love new control system rollouts

Willis1776 commented 4 years ago

I've noticed this as well and it's a logic processing (timing) issue in Rooms-Sources Controller.usp, specifically the UpdateWatchListen() function and how it populates the sources list arrays. I was able to resolve it by adding Delay(2); after both lines 711 (Watches_Size = Watches_Size + 1;) and 719 (Listens_Size = Listens_Size + 1). So below is what my UpdateWatchListen() looks like.

FUNCTION UpdateWatchListen()
{
    INTEGER i, room, source;
    room = IdToIndex(Room_Is);

    IF(room = 0)
    {
        RETURN;
    }

    IF(Room_Sources_Is[room] != "")
    {
        __Sources__ = Room_Sources_Is[room];
    }
    ELSE IF(Default_Available_Sources != "")
    {
        __Sources__ = Default_Available_Sources;
    }
    ELSE
    {
        __Sources__ = "";

        FOR(i = 1 TO #SOURCES_MAX)
        {
            IF(Source_Name_Is[i] != "")
            {
                MAKESTRING(__Sources__, "%s%c", __Sources__, IndexToId(i));
            }
        }
    }

    Watches_Size = 0;
    Listens_Size = 0;

    SETARRAY(__Watch_Source__, 0);
    SETARRAY(__Source_Watch__, 0);
    SETARRAY(__Listen_Source__, 0);
    SETARRAY(__Source_Listen__, 0);

    FOR(i = 1 TO LEN(__Sources__))
    {
        source = BYTE(__Sources__, i);
        source = IdToIndex(source);

        TRACE("UpdateWatchListen() source: %u", source);

        IF(Source_Has_Video[source])
        {
            Watches_Size = Watches_Size + 1;
            Delay(2);
            __Watch_Source__[Watches_Size] = source;
            __Source_Watch__[source] = Watches_Size;

            UpdateWatch(source);
        }
        ELSE
        {
            Listens_Size = Listens_Size + 1;
            Delay(2);
            __Listen_Source__[Listens_Size] = source;
            __Source_Listen__[source] = Listens_Size;

            UpdateListen(source);
        }
    }
}
sphere27 commented 4 years ago

Thank you

That is helpful advice that I'll try out. Have you noticed any other timing or performance issues related to using it on a 4 series processor?

I will try your suggestion and see what I run into. According to the job site, the list was also not populating the right source list for the right room. But I haven't caught it doing that.

I did however catch it not populating my rooms list at all one time, and also have seen it not populate the lights rooms list.

Thanks again

Jimi

On Thu, Jul 9, 2020, 11:24 AM Willis notifications@github.com wrote:

I've noticed this as well and it's a logic processing (timing) issue in Rooms-Sources Controller.usp, specifically the UpdateWatchListen() function and how it populates the sources list arrays. I was able to resolve it by adding Delay(2); after both lines 711 (Watches_Size = Watches_Size + 1;) and 719 (Listens_Size = Listens_Size + 1). So below is what my UpdateWatchListen() looks like.

FUNCTION UpdateWatchListen() { INTEGER i, room, source; room = IdToIndex(Room_Is);

IF(room = 0)
{
    RETURN;
}

IF(Room_Sources_Is[room] != "")
{
    __Sources__ = Room_Sources_Is[room];
}
ELSE IF(Default_Available_Sources != "")
{
    __Sources__ = Default_Available_Sources;
}
ELSE
{
    __Sources__ = "";

    FOR(i = 1 TO #SOURCES_MAX)
    {
        IF(Source_Name_Is[i] != "")
        {
            MAKESTRING(__Sources__, "%s%c", __Sources__, IndexToId(i));
        }
    }
}

Watches_Size = 0;
Listens_Size = 0;

SETARRAY(__Watch_Source__, 0);
SETARRAY(__Source_Watch__, 0);
SETARRAY(__Listen_Source__, 0);
SETARRAY(__Source_Listen__, 0);

FOR(i = 1 TO LEN(__Sources__))
{
    source = BYTE(__Sources__, i);
    source = IdToIndex(source);

    TRACE("UpdateWatchListen() source: %u", source);

    IF(Source_Has_Video[source])
    {
        Watches_Size = Watches_Size + 1;
        Delay(2);
        __Watch_Source__[Watches_Size] = source;
        __Source_Watch__[source] = Watches_Size;

        UpdateWatch(source);
    }
    ELSE
    {
        Listens_Size = Listens_Size + 1;
        Delay(2);
        __Listen_Source__[Listens_Size] = source;
        __Source_Listen__[source] = Listens_Size;

        UpdateListen(source);
    }
}

}

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/adelyte/crescendo/issues/100#issuecomment-656193178, or unsubscribe https://github.com/notifications/unsubscribe-auth/ANWWZVCEYTT2WVJW5PTDU4TR2XOMRANCNFSM4OVQYKMA .

Willis1776 commented 4 years ago

I haven't seen any other issues yet but I've only had one job with a 4 series processor. The job doesn't have that much in terms of "Extras", just a lot of Audio/Video. There's about 20 rooms with Video & Audio and then an additional 10 of Audio only so the Watch/Listen list is the main interaction.

Every now and then I'll see an "System_Initialize" issue on the first load of a project (happened with 3 series as well). I have a stepper that sends a "System_Reinitialize" after 5 seconds from initial project upload. That seems to have fixed that issue but definitely a bandaid type fix.

adelyte-chris commented 4 years ago

@Willis1776 Would you check if PROCESSLOGIC() resolves the issue as well as a DELAY(2)? I do not have a 4-Series processor available immediately to test against.

My guess is that there is a slight difference in how the 4-Series handles SIMPL+ analog output read backs (cf. the Crestron SIMPL+ Language Reference Guide p 48).

If it does work, PROCESSLOGIC() is generally preferable to DELAY(/* hsecs */)because it invokes a task switch with immediate reentry on the next logic wave rather than the posting to the realtime scheduler.

Let me know either way, and we'll get a fix committed. If you prefer to issue a pull request so that you get full credit, please do so.

Willis1776 commented 4 years ago

Yeah, I can definitely run those tests with PROCESSLOGIC() and let you know. If it works, I can put in a pull request for it.

Willis1776 commented 4 years ago

Ok, after doing some testing PROCESSLOGIC() doesn't appear to be doing what it was intended to do on a 4 series processor. The Watches_Size + 1 & Listens_Size + 1 does not get processed right away. Adding any kind of logic step after both of those commands will then allow everything to process correctly (even a TRACE() will cause everything to process correctly).

According to the documentation (If I'm reading it correctly) since Watches_Size and Listens_Size are ANALOG_OUTPUTS, the PROCESSLOGIC() should allow them to update and then the rest of the logic should process which it doesn't appear to be doing. Here's my updated function using local variables for the logic as well as updated the 2 ANALOG_OUTPUTS (Watches_Size & Listens_Size).

If there is a better way let me know. I'll submit a pull request with this change.


FUNCTION UpdateWatchListen()
{
    INTEGER i, room, source, watch_size, listen_size;

    room = IdToIndex(Room_Is);

    IF(room = 0)
    {
        RETURN;
    }

    IF(Room_Sources_Is[room] != "")
    {
        __Sources__ = Room_Sources_Is[room];
    }
    ELSE IF(Default_Available_Sources != "")
    {
        __Sources__ = Default_Available_Sources;
    }
    ELSE
    {
        __Sources__ = "";

        FOR(i = 1 TO #SOURCES_MAX)
        {
            IF(Source_Name_Is[i] != "")
            {
                MAKESTRING(__Sources__, "%s%c", __Sources__, IndexToId(i));
            }
        }
    }

    watch_size = 0;
    listen_size = 0;

    SETARRAY(__Watch_Source__, 0);
    SETARRAY(__Source_Watch__, 0);
    SETARRAY(__Listen_Source__, 0);
    SETARRAY(__Source_Listen__, 0);

    FOR(i = 1 TO LEN(__Sources__))
    {
        source = BYTE(__Sources__, i);
        source = IdToIndex(source);

        TRACE("UpdateWatchListen() source: %u", source);

        IF(Source_Has_Video[source])
        {
            watch_size = watch_size + 1;
            Watches_Size = watch_size;
            __Watch_Source__[watch_size] = source;
            __Source_Watch__[source] = watch_size;

            UpdateWatch(source);
        }
        ELSE
        {
            listen_size = listen_size + 1;
            Listens_Size = listen_size;
            __Listen_Source__[listen_size] = source;
            __Source_Listen__[source] =listen_size;

            UpdateListen(source);
        }
    }
}