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

Example for external messaging. #22

Closed
wants to merge 4 commits into from
Closed

Example for external messaging. #22

wants to merge 4 commits into from

Conversation

floitsch
Copy link
Member

No description provided.

@floitsch floitsch marked this pull request as draft September 18, 2023 14:40
Copy link
Member

@kasperl kasperl left a comment

Choose a reason for hiding this comment

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

Looks good to me.


test handler/MessageHandler data/ByteArray:
copy := data.copy // Data can be neutered as part of the transfer.
process_send_ EXTERNAL_PID TYPE data
Copy link
Member

Choose a reason for hiding this comment

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

Maybe it is worth abstracting a little bit over this. If you end up with a symbolic name for external message handlers then having a wrapper, so you can add a helper for sending messages and maybe for receiving them with a lambda instead of a full SystemMessageHandler_ would be nice.

You could have a --copy flag for the sending (default to true) like in the Artemis package: https://github.com/toitware/toit-artemis/blob/main/src/artemis.toit#L339.

main:
print "starting"
handler := MessageHandler
set_system_message_handler_ (TYPE + 1) handler
Copy link
Member

Choose a reason for hiding this comment

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

Kebabify?

@@ -0,0 +1 @@
# set(EXTRA_COMPONENT_DIRS ${EXTRA_COMPONENT_DIRS} ${CMAKE_CURRENT_SOURCE_DIR}/src)
Copy link
Member

Choose a reason for hiding this comment

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

What is this for?

static toit_err_t on_message(void* user_context, int sender, int type, void* data, int length);
static toit_err_t on_removed(void* user_context);

typedef struct process_t {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
typedef struct process_t {
typedef struct {


typedef struct process_t {
toit_process_context_t* process_context;
} process_t;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
} process_t;
} foo_context_t;

.on_message = &on_message,
.on_removed = &on_removed,
};
toit_err_t err = toit_set_callbacks(process_context, cbs);
Copy link
Member

Choose a reason for hiding this comment

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

This would be the natural place to provide the user_context.

printf("unable to allocate user context\n");
return;
}
toit_add_external_process(user_context, "toit.io/external-test", &start);
Copy link
Member

Choose a reason for hiding this comment

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

Maybe don't provide the user context just yet?

static toit_err_t start(void* user_context, toit_process_context_t* process_context) {
printf("starting process\n");
process_t* process = (process_t*)(user_context);
process->process_context = process_context;
Copy link
Member

Choose a reason for hiding this comment

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

It would be great if this was closer to the malloc.

@floitsch
Copy link
Member Author

Abandoning, as the code was integrated in other repositories/PRs.

@floitsch floitsch closed this May 14, 2024
@floitsch floitsch deleted the floitsch/custom_c branch May 14, 2024 09:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants