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

add no macros example, remove bogus print, fix crash on empty schedule #113

Merged
merged 5 commits into from
May 3, 2024

Conversation

wucke13
Copy link
Contributor

@wucke13 wucke13 commented Mar 12, 2024

No description provided.

Wanja Zaeske added 4 commits March 12, 2024 10:47
All our examples use the macros, but we should also demonstrate the ugly
truth behind those macros.
Add some comments, shorten imports a bit
This fixes a crash on the hypervisor, if no partition is scheduled.
Instead, in each partition window a warning is emitted, pointing out the
empty schedule for that partition window.
Comment on lines +11 to +17
a653rs = { workspace = true }
a653rs-postcard = { git = "https://github.com/DLR-FT/a653rs-postcard.git", branch = "main" }
a653rs-linux = { path = "../../partition" }
once_cell.workspace = true
serde = "1.0"
log = "0"
humantime = "2.1"
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should probably be workspace dependencies.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah ok for the example it is probably easier to copy-paste. But then again it should all not be workspace dependencies. Pick one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree. I just copied what was already there. I'm in favor of adjusting this, however, I think we should prepare an extra PR for that.

Comment on lines +5 to +12
//! - What is [`StartContext<Hypervisor>`]?
//! - Some parts of the ARINC 653 API are only available on a specific mode.
//! In particular, the process and port creationg API can only be called
//! during {`COLD`,`WARM`}`_START` mode (see [`OperatingMode`]).
//! `StartContext` is a struct that exposes this API only during
//! {`COLD`,`WARM`}`_START` mode, and can not be constructed by the user.
//! This makes misuse of the API (as in attempting to create a port/process
//! during `NORMAL`) impossible.
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should be part of the crate-level documentation and link to it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, actually, this should be part of the a653rs documentation.

anyhow!("At least one periodic or aperiodic process is expected to exist"),
));
let part_name = self.partition.base.name();
warn!("partition {part_name}: no process is scheduled")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Rather we should check at the end of cold_start if there is no process. If the partition is run multiple times a second, this produces too much log-spam on a relatively high log-level.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This then would be a check inside the partition, AFAICT. I'm not an expert on this however.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought further about this. I think it is completely fine to spam the log if there is a partition that at runtime creates an invalid configurration (no processes...). If anything, this should grab the attention of the developer, so spamming is justified. We could more clever logging (akin "20 identical messages dropped") but I don't see the justification for making this, considering that this only ever occurs when there is a low-key screw up by the developer to begin with.

@wucke13 wucke13 merged commit 9f6be0a into main May 3, 2024
10 checks passed
@dadada dadada deleted the dev/add-no-macros-example branch May 5, 2024 10:43
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