-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Updating the "Data Constructs: Commands, Events, Channels, and Parameters" Page in the F' User's Guide #2147
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Changed "pipes" to "ports" early in the Commands section, because I've never heard pipes used as a term in F Prime and ports make sense in the context of component-to-component communication.
Changed the last line two lines of "Command Dispatching" to more clearly reflect the operational flow described in the next section, "Command Sequencing"
Changed the 3rd sentence under "Events" to say that components defining events, rather than commands, should hook up to the Svc::ActiveLogger. This should maybe actually say "defining events AND commands"--I forget if the system always sends out an event for a completed command--but the log ports are definitely required for events
Changed the sixth sentence under the events header to be more parsable; this may also mean the sentence now communicates incorrect information, but it should definitely should looked at because it doesn't quite make sense right now
Changed the last lines under the "Event" to more accurately reflect where the different log output ports store their event logs, assuming that these binary and text log ports line up with the "Fw.Log" and "Fw.TextLog" ports respectively as described in the FPP User's Guide in section 9.3.2
Updated the second property under the header "Telemetry" to say that the name is prepended with the name of the component instance, and not just the component. This is both consistent with the other data constructs on this page, with how telemetry is usually packetized--"channel name="[componentInstance].[TELEMETRY_CHANNEL]""--and how telemetry coming from two different instances of the same component should intuitively be distinguishable.
Changed the second sentence under the "Telemetry Database" header for grammatical and semantic clarity.
Changed the last sentence in the figure 8 description to reflect how the parameter manager changes parameter values through the auto-generated set and save commands; my details might be a little fuzzy, so feel free to correct the specifics, but I think this section deserves a little more fleshing out. I'm also unsure of if the line before this should be expanded upon? Simply saying "Components can set and retrieve parameters" might appear rather opaque to folks who are newer to F' (like myself), even with the context of the Svc::PrmDb which one can look into further in the official F Prime repository.
LeStarch
requested changes
Aug 1, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One minor error. I'll fix it if I can.
LeStarch
approved these changes
Aug 1, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had permissions to correct the regression. This should be good to go!
thomas-bc
pushed a commit
that referenced
this pull request
Aug 4, 2023
…ters" Page in the F' User's Guide (#2147) * Update cmd-evt-chn-prm.md Changed "pipes" to "ports" early in the Commands section, because I've never heard pipes used as a term in F Prime and ports make sense in the context of component-to-component communication. * Update cmd-evt-chn-prm.md Changed the last line two lines of "Command Dispatching" to more clearly reflect the operational flow described in the next section, "Command Sequencing" * Update cmd-evt-chn-prm.md Changed the 3rd sentence under "Events" to say that components defining events, rather than commands, should hook up to the Svc::ActiveLogger. This should maybe actually say "defining events AND commands"--I forget if the system always sends out an event for a completed command--but the log ports are definitely required for events * Update cmd-evt-chn-prm.md Changed the sixth sentence under the events header to be more parsable; this may also mean the sentence now communicates incorrect information, but it should definitely should looked at because it doesn't quite make sense right now * Update cmd-evt-chn-prm.md Changed the last lines under the "Event" to more accurately reflect where the different log output ports store their event logs, assuming that these binary and text log ports line up with the "Fw.Log" and "Fw.TextLog" ports respectively as described in the FPP User's Guide in section 9.3.2 * Update cmd-evt-chn-prm.md Updated the second property under the header "Telemetry" to say that the name is prepended with the name of the component instance, and not just the component. This is both consistent with the other data constructs on this page, with how telemetry is usually packetized--"channel name="[componentInstance].[TELEMETRY_CHANNEL]""--and how telemetry coming from two different instances of the same component should intuitively be distinguishable. * Update cmd-evt-chn-prm.md Changed the second sentence under the "Telemetry Database" header for grammatical and semantic clarity. * Update cmd-evt-chn-prm.md Changed the last sentence in the figure 8 description to reflect how the parameter manager changes parameter values through the auto-generated set and save commands; my details might be a little fuzzy, so feel free to correct the specifics, but I think this section deserves a little more fleshing out. I'm also unsure of if the line before this should be expanded upon? Simply saying "Components can set and retrieve parameters" might appear rather opaque to folks who are newer to F' (like myself), even with the context of the Svc::PrmDb which one can look into further in the official F Prime repository. * Fixing minor regression --------- Co-authored-by: M Starch <LeStarch@googlemail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Change Description
Made general updates to the page from the F' User's Guide called "Data Constructs: Commands, Events, Channels, and Parameters", largely for clarity and grammatical correctness. Each change was placed in a separate commit, each with their own particular description.
Rationale
Some sentences were difficult to parse; some of the factually-provided information seemed to clash with other materials and my current understanding of F Prime. Again, each change was placed in a separate commit, and specific rationales were attached to each of them.
Testing/Review Recommendations
N/A
Future Work
The last commit I made, which dealt with how parameters are set and retrieved by parameters, was a suggested change but a subject that I, myself, still don't understand 100%; so, while I made an educated guess on a fleshed out description, this is likely something that deserves some clarifying somewhere. If I'm missed a page where it is explored in more detail, apologies, and feel free to point me to it.