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

Ringlogger Enhancements: Multiple Application Filter Support, Argument Support to RingLogger Launch Command, Filter Support For Date Navigation #196

Open
wants to merge 24 commits into
base: main
Choose a base branch
from

Conversation

Shivaji-Dhepale
Copy link

This PR introduces several enhancements to the Ringlogger viewer command systems, improving its flexibility and functionality:

1.Enhancement to existing application list command:
Now application command can accept a list of applications, which allows more dynamic interaction with multiple applications at once.

2.Optional Initial Command String for Launch Command:
Launch command is modified to accept string as an argument, enabling users to pass additional parameters as command string directly during launch.
Additional command string can be as follows:
1. Ringlogger.viewer("l debug")
2. Ringlogger.viewer("a app_one app_two app_three")
3. Ringlogger.viewer("d 2025-12-11 10:20:11")
4. Ringlogger.viewer("r")
5. Ringlogger.viewer("g grab_some_data")
6. Ringlogger.viewer("q")

3.Addition of New Command for Date Entry Navigation:
A new command has been added to facilitate easy navigation to the date entry section. This improves the overall user experience when working with time-sensitive data.

These changes aim to provide greater control and customisation for users interacting with the Ringlogger command system, enhancing both usability and diagnostic capabilities.

Testing:

  1. Unit Test cases are updated for these enhancements
  2. Conducted manual testing to validate the functionality of the newly introduced commands, ensuring they work as expected in various scenarios.
  3. Verified that the launch command correctly accepts and processes optional arguments while maintaining full backward compatibility.
  4. Confirmed that the new date entry navigation command accurately redirects to the date entry section, enhancing usability for time-sensitive operations

Demo:
command: > a nil $kmsg telit_modem
Screenshot from 2025-01-28 18-37-57
Response:
Screenshot from 2025-01-28 18-37-36

command: Ringlogger.viewer("l info; a telit_modem vintage_net")
FAQM-1582-launch-command-with-args 2
Response:
launch-command-test-preview

command: > d 2025-01-06 05:54:10
goto-date-command
Response:
goto-date-test-preview

@taun-fellowes
Copy link
Contributor

Hi @fhunleth, FYI, this is part of Fellowes Nerves work. We are not yet using these "enhancements" as we would like your feedback and review before we start committing the muscle memory. :-)

These are mostly about efficiency both in terms of bandwidth and IoT device processing. For example, if the developer knows they want to filter based on 3 apps, doing it once is 1/3rd the work as applying the filter once for each appended app filter. Similar thinking in being able to pass in the desired filter as part of the viewer startup.

Lastly, we have a use case where a less technical person is responsible for using the viewer and gathering troubleshooting information. These enhancements make it easier to get them there faster.

@fhunleth
Copy link
Member

Hi @taun-fellowes and @Shivaji-Dhepale! These improvements actually look really nice. I'm in the process of giving them a try in between meetings and work. I was going to give some feedback after I got a chance to look at it more since it's a bit of a context switch at the moment for me.

In the mean time, would you mind rebasing on main just in case there are little Elixir 1.18, formatter, or credo things to address.

Suraj Borate and others added 19 commits January 29, 2025 10:38
FAQM-1743: Rebased Fellowes Ringlogger Fork with Upstream and Verified Changes
Bumps [credo](https://github.com/rrrene/credo) from 1.7.7 to 1.7.9.
- [Release notes](https://github.com/rrrene/credo/releases)
- [Changelog](https://github.com/rrrene/credo/blob/master/CHANGELOG.md)
- [Commits](rrrene/credo@v1.7.7...v1.7.9)

---
updated-dependencies:
- dependency-name: credo
  dependency-type: direct:development
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Bumps [credo](https://github.com/rrrene/credo) from 1.7.9 to 1.7.10.
- [Release notes](https://github.com/rrrene/credo/releases)
- [Changelog](https://github.com/rrrene/credo/blob/master/CHANGELOG.md)
- [Commits](rrrene/credo@v1.7.9...v1.7.10)

---
updated-dependencies:
- dependency-name: credo
  dependency-type: direct:development
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Bumps [dialyxir](https://github.com/jeremyjh/dialyxir) from 1.4.3 to 1.4.5.
- [Release notes](https://github.com/jeremyjh/dialyxir/releases)
- [Changelog](https://github.com/jeremyjh/dialyxir/blob/master/CHANGELOG.md)
- [Commits](jeremyjh/dialyxir@1.4.3...1.4.5)

---
updated-dependencies:
- dependency-name: dialyxir
  dependency-type: direct:development
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Bumps [ex_doc](https://github.com/elixir-lang/ex_doc) from 0.34.2 to 0.35.0.
- [Release notes](https://github.com/elixir-lang/ex_doc/releases)
- [Changelog](https://github.com/elixir-lang/ex_doc/blob/main/CHANGELOG.md)
- [Commits](elixir-lang/ex_doc@v0.34.2...v0.35.0)

---
updated-dependencies:
- dependency-name: ex_doc
  dependency-type: direct:development
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Bumps [ex_doc](https://github.com/elixir-lang/ex_doc) from 0.35.0 to 0.36.0.
- [Release notes](https://github.com/elixir-lang/ex_doc/releases)
- [Changelog](https://github.com/elixir-lang/ex_doc/blob/main/CHANGELOG.md)
- [Commits](elixir-lang/ex_doc@v0.35.0...v0.36.0)

---
updated-dependencies:
- dependency-name: ex_doc
  dependency-type: direct:development
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Bumps [ex_doc](https://github.com/elixir-lang/ex_doc) from 0.36.0 to 0.36.1.
- [Release notes](https://github.com/elixir-lang/ex_doc/releases)
- [Changelog](https://github.com/elixir-lang/ex_doc/blob/main/CHANGELOG.md)
- [Commits](elixir-lang/ex_doc@v0.36.0...v0.36.1)

---
updated-dependencies:
- dependency-name: ex_doc
  dependency-type: direct:development
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Bumps [credo](https://github.com/rrrene/credo) from 1.7.10 to 1.7.11.
- [Release notes](https://github.com/rrrene/credo/releases)
- [Changelog](https://github.com/rrrene/credo/blob/master/CHANGELOG.md)
- [Commits](rrrene/credo@v1.7.10...v1.7.11)

---
updated-dependencies:
- dependency-name: credo
  dependency-type: direct:development
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
This aligns the official supported Elixir versions with other Nerves
packages.
This is a quick bandaid to make persistence tests less susceptible to
issues from every test using the same log file.
@Shivaji-Dhepale
Copy link
Author

Hi @fhunleth, We rebased our branch to main on January 29, 2025. Also resolved the CircleCI pipeline failures. Requesting you to review it and let us know if any changes are required.

@fhunleth
Copy link
Member

@Shivaji-Dhepale Thanks for the note. I think there is a misunderstanding on what I was looking for. I really need the PR to be much smaller so that it's easier to review. It has unrelated changes in the commit history, and GitHub is hopefully showing you the warning about the "branch cannot be rebased safely".

I use the git commandline tools, so for rebasing to main involves git fetch origin main and then a git rebase origin/main. Maybe you refer to this repository as upstream or something similar. After I rebased, this PR looked like main...FAQM-1585-concat-commands-support. It has 7 commits vs. the 24 in this PR. Git will also allow me to merge those 7 commits. Combined with CI passing that's the minimum I need to really start with the review.

I did start reviewing and there's a lot. I like the features, but if we review all at once, I am worried that it will never be merged. Splitting this up into smaller PRs will be much quicker. You had a good list for PRs:

  1. Enhancement to existing application list command
  2. Optional Initial Command String for Launch Command
  3. Addition of New Command for Date Entry Navigation

The date entry command might be an easy one to start with since it seems the smallest.

If you're up for it, ping me on the Elixir slack and perhaps we can set up a time to pair on getting these changes in.

@Shivaji-Dhepale
Copy link
Author

As per your suggestion, we have created a PR for the date entry navigation feature.
We have added an additional function, parse_launch_cmd/2, explicitly to successfully execute test cases for all the features to be implemented.
Once you approve and merge this PR, we will create additional PRs for other features such as:

1.Enhancement to the existing application list command
2.Optional initial command string for the launch command
3.Command concatenation inside the viewer terminal

Hope this is in line with your expectations.
Thanks!

@fhunleth
Copy link
Member

Thanks! I saw the PR. I'll get to it soon.

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