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

The SW5 changes to protobuf along with futureproofing from the dev team #12

Merged
merged 47 commits into from
Aug 23, 2023

Conversation

seblund
Copy link
Member

@seblund seblund commented Feb 9, 2023

Changes the protobuf to be more future proof, should contain most things to support future features

mads256h and others added 30 commits October 5, 2022 11:19
Changed query response to either return a query if the userid is
accepted, or send an error along with the ID.
Removed optional keyword due to default settings in proto3
…simulation

Change simulation api to support a stateless solution
Added reason on results for some detail
 - Remove unused messages
 - Change "string component_name" to "SpecificComponent specific_component" in Location
 - Unest Location from LocationTuple
- Remove and replace StateTuple with State
Added the changes discussed in GRPC comittee
#11)

* Added action string to determinism, consitency and refinement queries.

* Changed refinement action from 4 to 5.
* query

* fix optional

* proto

* proto

* init

* name change
* query

* fix optional

* proto

* proto

* init

* name change

* Refactor

* Refactor

* Changed name
* query

* fix optional

* proto

* proto

* init

* name change

* Refactor

* Refactor

* Changed name

* refactor

* Added comments

* Refactor
Puvikaran2001 and others added 11 commits November 18, 2022 10:40
This pull request makes some changes to the reachability result.

Instead of result a list of edges as a path for each component combined, we have made a new message called Path that represents the path from a start state to an end state for the individual component.

Therefore in the message ReachabilityResult, we result a list of paths - one path for each individual component.

Co-authored-by: Mati-AAU <[email protected]>
Simulation user id and nondeterminism for ambiguous edge choices.
This was broken because it required Reveaal to store all the user tokens
that is has given out. This violates the principle of idempotency. The
server cannot know when a user id is no longer in use, because we have
no "log out" endpoint and even if we had one the client could crash and
thus not send out a "log out" message.

Instead we assume that all clients are well behaved and use GetUserToken
to get a token that is not in use.
action is now a repeated string, not a string!
* query

* fix optional

* proto

* proto

* init

* name change

* Refactor

* Refactor

* Changed name

* refactor

* Added comments

* Refactor

* Clock heights

* Naming

* Naming
…uotient (Universal and Error in quotient require it)
@Nielswps Nielswps mentioned this pull request Mar 6, 2023
seblund added 2 commits March 10, 2023 14:04
Update query failures and support simulation of systems with quotients
@seblund seblund requested a review from Nielswps March 10, 2023 14:55
Copy link
Contributor

@Nielswps Nielswps left a comment

Choose a reason for hiding this comment

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

Just some minor thoughts, looks great!

I am still kind of bumped about versioning, even though (hopefully) nothing will break. We will just create a GitHub release when this is merged, as discussed. It will be nice to have the ability to reference what version is currently used in the README.mds for future developers, even though the code itself is oblivious.

objects.proto Outdated
message SystemClock {
uint32 clock_index = 1;
}
message Zero {}
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we rename this to ZeroClock for consistency? Its purpose is pretty obvious in the way it is used, but maybe it would be nice to make it explicit

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree, it has been changed now :)

objects.proto Outdated
// When a clock is not attached to a component
SystemClock system_clock = 2;
// The zero clock
Zero zero = 3;
Copy link
Contributor

Choose a reason for hiding this comment

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

See comment above

objects.proto Outdated
Comment on lines 52 to 54
message Federation {
Disjunction disjunction = 1;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Is a Federation just a wrapper for a disjunction?

Copy link
Member Author

@seblund seblund Mar 10, 2023

Choose a reason for hiding this comment

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

Yes, I have changed it to use a disjunction instead now. It's also much nicer to just use a disjunction because it is more general, e.g. if we switch to CDDs in the future, it still makes sense to represent them by a disjunction.

Copy link
Contributor

@Nielswps Nielswps left a comment

Choose a reason for hiding this comment

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

Very nice! I will hopefully get started on merging the refactoring and these changes into the simulation branch of the GUI repo sometime next week ✌️

query.proto Outdated Show resolved Hide resolved
Co-authored-by: Thomas Lohse <[email protected]>
Copy link
Contributor

@t-lohse t-lohse left a comment

Choose a reason for hiding this comment

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

LGTM

@t-lohse t-lohse merged commit 053d7e9 into main Aug 23, 2023
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.