-
Notifications
You must be signed in to change notification settings - Fork 67
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
Dump system info and processing status to file #119
Conversation
@dubrsl @alexeysirenko @redradist Please provide your comments guys. |
At the moment system properties and processing status is not exposed to client software. Some go as far as parsing the web gui page to figure out this information. Lets make thier life easier and dump potentially useful information to json file, which can also be accessed over web. This PR addresses krakenrf#108.
ee542fd
to
e5df8c9
Compare
@godsic LGTM |
Sorry, but add dependency to SCM git in application is bad idea.
I suggest simply exporting the manually set value. The value store in a static configuration file, which will be updated when a new release is released. |
self.module_receiver.iq_header.cpi_length / self.module_receiver.iq_header.sampling_freq | ||
) * 1e3 | ||
daq_status["dropped_frames"] = self.dropped_frames | ||
|
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.
Might make sense adding a general status. By checking which you can see whether the receiver is ready or not.
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.
Fair enough, added daq_ok
.
@dubrsl Thanks for your feedback. Unfortunately, I cannot follow the first point, since the project opted for git as SCM. If you are talking about somebody who might have cloned it and turned into another SCM (e.g., Mercurial), then I don't think we should support that case. Regarding second one, I see your point for sure. Nevertheless I am not a fan of such manual steps, especially, given the fact that some of us run |
Ok.
I'm sure no one installs manually. And uses scripts https://github.com/krakenrf/krakensdr_docs/tree/main/install_scripts or Dockerfile. Therefore, I suggest setting the version and git hash in _ui/_web_interface/variables.py using this script. And remove the dependency on git. |
@dubrsl I now handle cases of missing |
7e1d3c8
to
ccf8a83
Compare
@@ -65,7 +65,7 @@ def get_system_control_card_layout(): | |||
], | |||
className="field", | |||
), | |||
html.Div("Version 1.7.0"), | |||
html.Div(f"Version {SOFTWARE_VERSION} ({SOFTWARE_GIT_HASH})"), |
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.
Using built metadata with version should be used via "+" and dot separated identifiers.
https://semver.org/#spec-item-10
Propose
{SOFTWARE_VERSION}+git.{SOFTWARE_GIT_HASH}
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.
@dubrsl Fixed according to your suggestion, but with short hash instead of full one.
Also use unique enough short hash instead of full one.
More thoughts. Is it possible to run a custom script(s) and include the result(s) it provides in the status? |
Most of those metrics can be retrieved using python built-in or 3rd party modules. I am happy to add those in followup. |
At the moment system properties and processing status is not exposed to client software. Some go as far as parsing the web gui page to figure out this information. Lets make thier life easier and dump potentially useful information to json file, which can also be accessed over web. This PR addresses #108.
gitpython
module is now recommended.