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

ASL3 USB Configs #282

Merged
merged 6 commits into from
Mar 9, 2024
Merged

ASL3 USB Configs #282

merged 6 commits into from
Mar 9, 2024

Conversation

tsawyer
Copy link
Member

@tsawyer tsawyer commented Feb 12, 2024

Update USB configs for moved tune settings. Add comments.

Update USB configs for moved tune settings. Add comments.
@Allan-N
Copy link
Collaborator

Allan-N commented Feb 12, 2024

I would like to suggest a few changes to the .conf files.

  1. The "[1999]" on line 19 be changed to [device-main](!)
  2. Above the ";;;;; ASL3 Tune settings..." on line 80 we add [1999](device-main)

This makes it simple to add a new node to the config as all we need is to add [node#](device-main) to the end of the file (vs. having to replicate all of the settings above).

@InterLinked1
Copy link
Member

I would like to suggest a few changes to the .conf files.

  1. The "[1999]" on line 19 be changed to [device-main](!)
  2. Above the ";;;;; ASL3 Tune settings..." on line 80 we add [1999](device-main)

This makes it simple to add a new node to the config as all we need is to add [node#](device-main) to the end of the file (vs. having to replicate all of the settings above).

Thanks for the feedback @Allan-N - in the future, it's often useful to provide feedback using the "review" functionality since you can make those comments inline, and the PR will then show what changes need to be addressed for merging with some basic resolution tracking built in.

It's not very obvious - I'm only familiar with this now that the Asterisk project has moved to GitHub and makes us do it that way.

@Allan-N
Copy link
Collaborator

Allan-N commented Feb 12, 2024

Yes, learning how to do things the GitHub way. I'll add review comments when I am back home ... if changes have not yet been made.

@tsawyer
Copy link
Member Author

tsawyer commented Feb 12, 2024

I don't see an advantage to using a template for USB configurations.

  1. There are many manufactures and many different radios making for no template that would satisfy more than a small minority of configurations.
  2. There are a dozen or so settings of which better than half are likely to change for any given node.
  3. The existing simple-tune-menu and radio-tune-menu currently read and write both the old and the new configs. ASL3-menu won't change any settings other than the node number and the device string.

Btw, line 80 would not be the right place for [node#](device-main). Just above #tryinclude custom/simpleusb.conf would be where it goes.

@Allan-N
Copy link
Collaborator

Allan-N commented Feb 12, 2024

@tsawyer

Advantages :

  • IF a system has more than one node then using a template eliminates the need to populate the configuration for each device with the same basic settings.
  • IF one wants to add a new node/device to then the changes could be as simple as adding a [1234](device-main) line.
  • Removing a node/device from simpleusb.conf is also simple (e.g. delete from [1234] to the next [).
  • Using templates for the node/device configuration GREATLY simplifies the changes that node-setup would need to make to simpleusb.conf, usbradio.conf, voter.conf, etc.

Disadvantages :

  • Aside from the configuration file being a bit "different" from the past (and we're already changing things by merging in the "tune" settings), I don't see any reason NOT to use the templates.

Re: simple-tune-menu

  • No changes should be needed for forward/backward compatibility because the settings go into the [1234] category.

Re: line #'s

  • I thought I got the line #'s correct for "simpleusb.conf" (line 19 in the revised diffs to define the template, line 80 where the per-node/device changes start).

Copy link
Collaborator

@Allan-N Allan-N left a comment

Choose a reason for hiding this comment

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

I would like us to adopt templates for "simpleusb.conf", "usbradio.conf", "voter.conf", ...

configs/rpt/simpleusb.conf Show resolved Hide resolved
configs/rpt/simpleusb.conf Show resolved Hide resolved
@tsawyer
Copy link
Member Author

tsawyer commented Feb 12, 2024

@Allan-N presented advantages are counter to the points @tsawyer made above. Tempelate proposal not accepted.

More updates for tune settings.
@tsawyer
Copy link
Member Author

tsawyer commented Feb 14, 2024

@InterLinked1 How do I nix Allan's suggestions?

Copy link
Member

@InterLinked1 InterLinked1 left a comment

Choose a reason for hiding this comment

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

Do configs/samples/simpleusb.conf.sample and usbradio.conf.sample also need to be updated?
The overall format looks somewhat different to me, but maybe somebody that understands the config can answer.

@tsawyer
Copy link
Member Author

tsawyer commented Feb 15, 2024

Do configs/samples/simpleusb.conf.sample and usbradio.conf.sample also need to be updated? The overall format looks somewhat different to me, but maybe somebody that understands the config can answer.

I'll have a look.
[edit]
I've never looked at those before. @KB4MDD do these make any sense to you?

Use Jim's/ASL call. Menu will be able to change it.
Copy link
Member

@InterLinked1 InterLinked1 left a comment

Choose a reason for hiding this comment

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

I'm still curious about if the sample files need any updates or not.

@tsawyer
Copy link
Member Author

tsawyer commented Feb 21, 2024

I'm still curious about if the sample files need any updates or not.

They do, but can we make that a separate issue? We need to move ahead on the asl3-menu and other testing.

@InterLinked1
Copy link
Member

I'm still curious about if the sample files need any updates or not.

They do, but can we make that a separate issue? We need to move ahead on the asl3-menu and other testing.

I'm kind of skeptical it will get done, then. Maybe @KB4MDD knows?

@KB4MDD
Copy link
Collaborator

KB4MDD commented Feb 21, 2024

The samples and rpt directory file changes need to push with this update.

This update is going to break everyone that is testing. This is simply unavoidable. The example files need to go out with this change.

I have already updated my simpleusb.conf and usbradio.conf in anticipation of this change.

If I need to update the samples, I will do that so that all of this can merge at the same time.

This updates the rpt and sample configs with the current options.
Remove USB tune files as settings have moved into their respective main conf file.
@InterLinked1 InterLinked1 merged commit 9eaab3c into master Mar 9, 2024
2 checks passed
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.

4 participants