Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wiredness with source lists on MC4 #100

Open
sphere27 opened this issue Jul 9, 2020 · 6 comments · May be fixed by #101
Open

Wiredness with source lists on MC4 #100

sphere27 opened this issue Jul 9, 2020 · 6 comments · May be fixed by #101

Comments

@sphere27
Copy link

sphere27 commented Jul 9, 2020

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
Copy link

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
Copy link
Author

sphere27 commented Jul 9, 2020 via email

@Willis1776
Copy link

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
Copy link
Member

@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
Copy link

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
Copy link

Willis1776 commented Jul 9, 2020

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);
        }
    }
}

@Willis1776 Willis1776 linked a pull request Jul 9, 2020 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants