Implement new UI for selecting signals #189
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
Signed-off-by: Ivan Čukić <[email protected]>
Signed-off-by: Ivan Čukić <[email protected]>
Signed-off-by: Ivan Čukić <[email protected]>
Signed-off-by: Ivan Čukić <[email protected]>
vimpostor
force-pushed
the
ivan/new-signal-selection-ui
branch
from
a33e03f
to
6e5c98a
Compare
Signed-off-by: Magnus Groß <[email protected]>
vimpostor
force-pushed
the
ivan/new-signal-selection-ui
branch
from
6e5c98a
to
803e2e2
Compare
Signed-off-by: Magnus Groß <[email protected]>
The CI complains about it, even though it runs gcc14 already. Signed-off-by: Magnus Groß <[email protected]>
Signed-off-by: Magnus Groß <[email protected]>
Signed-off-by: Ivan Čukić <[email protected]>
Signed-off-by: Magnus Groß <[email protected]>
Also generate about 100k signals for testing. Signed-off-by: Magnus Groß <[email protected]>
Signed-off-by: Ivan Čukić <[email protected]>
RalphSteinhagen
approved these changes
Jul 30, 2024
There was a problem hiding this comment.
Thanks a lot for the UI design implementation.
The selector works/looks functionally OK. There are some immediate follow-up items/issues (thanks for @ivan-cukic for documenting):
- the columns are not filled with the correct test mock-information information (minor) -> will follow this up afterwards
- [2SP] UI: Extend subscription to support not only STREAMING data #194
- [0.5SP,0.5SP] UI: Button to add signals and show them immediately in a chart #195
- [0.5SP,0.5SP] UI: Allow selecting multiple signals to add at the same time #193
and some that could/should be followed up afterwards, namely: - [1SP] UI: Move time consuming operations off of the main UI thread #190
- [0.5SP,0.5SP] UI: Mouse press on a block in the flow graph should not open settings panel #192
- [0.5SP] UI: Show error messages to the user #191
but which should not prevent the merging of this implementation. 👍
| dns::Entry{ "mdp", *restUrl.hostName(), 12345, "/GnuRadio/Acquisition", "", entry.name, entry.unit, entry.sample_rate, "STREAMING" }, | ||
| dns::Entry{ "mds", *restUrl.hostName(), 12345, "/GnuRadio/Acquisition", "", entry.name, entry.unit, entry.sample_rate, "STREAMING" } | ||
| dns::Entry{*restUrl.scheme(), *restUrl.hostName(), *restUrl.port(), "/GnuRadio/Acquisition", "", entry.name, entry.unit, entry.sample_rate, "STREAMING"}, | ||
| // dns::Entry{"mdp", *restUrl.hostName(), 12345, "/GnuRadio/Acquisition", "", entry.name, entry.unit, entry.sample_rate, "STREAMING"}, |
There was a problem hiding this comment.
We should make this dependent on whether the UI is run on the web or natively (i.e. within the browser or desktop).
| void Block::setDatatype(DataType type) { m_datatype = type; } | ||
| DataType Block::datatype() const { return m_datatype; } | ||
| void Block::setCurrentInstantiation(const std::string& newInstantiation) { | ||
| m_inputs.clear(); |
There was a problem hiding this comment.
to note: should follow GR4 naming conventions -> separate issue.
|
|
||
| class SignalSelector { | ||
| private: | ||
| std::string m_windowName = "Add Device Signals"; |
There was a problem hiding this comment.
follow GR4 naming conventions -> separate issue
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.
Closes #188
Set
OPENDIGITIZER_LOAD_TEST_SIGNALS=1for testing.The rendered columns are the name and comment for now, but can be changed if desired.
signals.webm