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

port: add helper function for parsing message tuple #997

Merged
merged 2 commits into from
Dec 26, 2023

Conversation

bettio
Copy link
Collaborator

@bettio bettio commented Dec 23, 2023

Add function for hiding port message tuple format details, so code doesn't need to hardcode indexes or repeat always the same checks, that can be instead shared across different port drivers.

These changes are made under both the "Apache 2.0" and the "GNU Lesser General
Public License 2.1 or later" license terms (dual license).

SPDX-License-Identifier: Apache-2.0 OR LGPL-2.1-or-later

@bettio bettio force-pushed the change-port-call-tuple branch from a2cc409 to cf1fbdd Compare December 24, 2023 00:35
@bettio bettio marked this pull request as draft December 24, 2023 00:35
@bettio bettio force-pushed the change-port-call-tuple branch 2 times, most recently from 1b313b1 to 47c8c60 Compare December 25, 2023 09:31
@bettio bettio marked this pull request as ready for review December 25, 2023 09:32
@bettio bettio force-pushed the change-port-call-tuple branch 2 times, most recently from 4332144 to 7358e20 Compare December 25, 2023 09:38
@bettio bettio changed the title port: use same message format that is used for gen_server port: change message tuple fmt and add new helper function Dec 25, 2023
@bettio bettio changed the title port: change message tuple fmt and add new helper function port: change message tuple fmt and add helper function Dec 25, 2023
@bettio bettio force-pushed the change-port-call-tuple branch 3 times, most recently from 17fb7de to 14197e2 Compare December 25, 2023 11:32
Remove duplicate code and add checks.

Signed-off-by: Davide Bettio <davide@uninstall.it>
Future proof #2: return the type of message instead of a generic Ok
value. Right now we support only calls.

Signed-off-by: Davide Bettio <davide@uninstall.it>
@bettio bettio force-pushed the change-port-call-tuple branch from 1cfd28b to 804ff69 Compare December 25, 2023 15:11
@bettio bettio changed the title port: change message tuple fmt and add helper function port: add helper function for parsing message tuple Dec 25, 2023
Copy link
Collaborator

@fadushin fadushin left a comment

Choose a reason for hiding this comment

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

LGTM


term pid;
term ref;
} GenMessage;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not all messages will have a ref or even a pid. Are we okay with this? I am fine with it, and I consider this a kind of interim solution, so that's fine. Just thinking ahead to what an $info or $cast message would look like.

This is just an observation, not a change request.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In that case GenCastMessage instead of GenCallMessage will be returned (pid and ref will be invalid).

@bettio bettio merged commit 564dcc6 into atomvm:master Dec 26, 2023
85 checks passed
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.

2 participants