Skip to content

Conversation

@chrisguikema
Copy link
Contributor

This commit adds a helper function to get a client structure based on the badge. The allows for clients to be numerical, but not sequential, and guarantees serial input will always align to the correct client on each build.

A side-effect of this method is that the clients are switched to by typing the ESCAPE_CHAR + badge number. This limits the number of clients to single-digits, 0 -> 9.

This commit adds a helper function to get a client structure based on
the badge. The allows for clients to be numerical, but not sequential,
and guarantees serial input will always align to the correct client on
each build.

A side-effect of this method is that the clients are switched to by
typing the ESCAPE_CHAR + badge number. This limits the number of clients
to single-digits, 0 -> 9.

Signed-off-by: Chris Guikema <chris.guikema@dornerworks.com>
@chrisguikema
Copy link
Contributor Author

From previous PR:

"Decoding badge numbers into getchar client IDs isn't safe unless client ID value 0 is reserved, which this change doesn't appear to be doing. The simplest approach that I can think of is to allow a string to be assigned to each client in the connection configuration (or just take the name of the instance+interface) and then allocate the getchar client IDs based on a lexicographical order of all the clients. This should provide a stable ID assignment across builds, and still allow badges to be allocated internally (where it can be ensured that badge 0 isn't used)."

This hasn't been addressed yet, but I will handle it properly.

@chrisguikema
Copy link
Contributor Author

@kent-mcleod I think i've addressed the zero badge issue. I guess it was an issue before this PR, so if you want to recommend any other changes, I'll be happy to implement.

I also updated the corresponding PR in camkes-vm to align each serial server connection.

@kent-mcleod
Copy link
Member

@kent-mcleod I think i've addressed the zero badge issue. I guess it was an issue before this PR, so if you want to recommend any other changes, I'll be happy to implement.

I don't expect that it was an issue before? The templates were generating badges >= 1 and then the init logic was assigning them to clients based on enumeration order rather than badge value.

    /* query what getchar clients exist */
    if (getchar_num_badges) {
        num_getchar_clients = getchar_num_badges();
        getchar_clients = calloc(num_getchar_clients, sizeof(getchar_client_t));
        for (int i = 0; i < num_getchar_clients; i++) {
            unsigned int badge = getchar_enumerate_badge(i);

(Although, if you manually set the badge to 0 via the *_attributes setting in the camkes configuration, then you could get a client with a badge value of 0).

@chrisguikema
Copy link
Contributor Author

I more meant that the putchar interfaces would simply use the raw badge number, so if the badge was always > 1, then the first color in the color array wouldn't be used?

@chrisguikema
Copy link
Contributor Author

Yeah, I just tested without these changes, and Green/Blue were the colors being used (not Red). And the colors used by the VMM and VMs were inverted, which could be a bit confusing.

@chrisguikema chrisguikema force-pushed the serial_server_client_badges branch 3 times, most recently from 1c2157a to a718047 Compare November 3, 2022 16:13
A zero badge is technically an unbadged notification, which would allow
the owner to distribute the capability. CAmkES makes sure the attributes
aren't set to zero, but that meant that "Client 0" was unused.

This commit initializes the putchar interfaces, and subtracts 1 from the
badge to get a client ID. This adds a runtime check to ensure a badge
of 0 cannot be used by the serial server.

Signed-off-by: Chris Guikema <chris.guikema@dornerworks.com>
@chrisguikema chrisguikema force-pushed the serial_server_client_badges branch from a718047 to a4b933b Compare November 3, 2022 17:48
This commit does two things. One, it uses CAmkES to automatically
generate the function headers. Two, it adds a template with a WEAK
version of the headers, since if an interface isn't connected, the
headers will not generate.

Signed-off-by: Chris Guikema <chris.guikema@dornerworks.com>
@chrisguikema chrisguikema force-pushed the serial_server_client_badges branch from a4b933b to 4353bdf Compare November 3, 2022 17:58
@kent-mcleod kent-mcleod requested a review from axel-h November 4, 2022 21:34

#define ESCAPE_CHAR '@'
#define MAX_CLIENTS 12
#define MAX_CLIENTS 10
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a note in the code here, what drives this limit? I think it would help to get the background for people new to the code. Increasing this is not trivially possible.

}

static int active_client = 0;
static int active_client = -2;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe make -1 and -2 a dedicated define now, so this is a bit more transparent, what magic values exist.

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 this pull request may close these issues.

3 participants