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.

Sign up for GitHub

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

Jump to bottom

Add ARDOP dial bandwidth #332

Merged
merged 1 commit into from
Apr 21, 2022
Merged

Add ARDOP dial bandwidth #332

merged 1 commit into from
Apr 21, 2022

Conversation

xylo04
Copy link
Contributor

@xylo04 xylo04 commented Mar 14, 2022

This will also be needed for VARA.

@xylo04
Copy link
Contributor Author

xylo04 commented Mar 14, 2022

UI screenshot:
image

The bandwidth field could be a little wider, but I'm not sure I want to take the space away from frequency when bandwidth only applies to one transport, soon two.

martinhpedersen
martinhpedersen reviewed Mar 15, 2022
View reviewed changes
Copy link
Member

@martinhpedersen martinhpedersen left a comment

Choose a reason for hiding this comment

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

This has been on my wishlist for some time now 🎉 (Resolves #120)

I agree with the input width, let's keep it simple for now 👍 However, I have some concerns regarding the code design. It should be clear when you read the inline comments.

Let's keep working on this! 😄

connect.go Outdated Show resolved Hide resolved
connect.go Outdated Show resolved Hide resolved
connect.go Outdated Show resolved Hide resolved
rmslist.go Outdated Show resolved Hide resolved
@xylo04 xylo04 marked this pull request as draft March 16, 2022 02:55
@xylo04
Copy link
Contributor Author

xylo04 commented Mar 18, 2022

This is dependent on la5nta/wl2k-go#73, but take a look when you're ready.

@xylo04 xylo04 force-pushed the ardop-bandwidth branch 3 times, most recently from 7109104 to 7d29800 Compare March 21, 2022 22:26
@xylo04 xylo04 self-assigned this Mar 25, 2022
@xylo04 xylo04 force-pushed the ardop-bandwidth branch 3 times, most recently from c28ac51 to 4a1ce10 Compare March 27, 2022 16:05
@xylo04 xylo04 mentioned this pull request Mar 29, 2022
Merged
@xylo04 xylo04 force-pushed the ardop-bandwidth branch 6 times, most recently from 9140724 to 27aa69d Compare April 5, 2022 03:16
martinhpedersen added a commit that referenced this pull request Apr 9, 2022
@martinhpedersen
Pull wl2k-go v0.9.2 (ARDOP dial bandwidth)
99d2600 
Ref #120
Ref #332
martinhpedersen
martinhpedersen reviewed Apr 9, 2022
View reviewed changes
web/dist/index.html Outdated Show resolved Hide resolved
@xylo04 xylo04 marked this pull request as ready for review April 10, 2022 03:29
@xylo04 xylo04 force-pushed the ardop-bandwidth branch 3 times, most recently from 30c4876 to c429fb9 Compare April 10, 2022 14:05
@martinhpedersen martinhpedersen self-requested a review April 12, 2022 09:43
martinhpedersen
martinhpedersen reviewed Apr 12, 2022
View reviewed changes
connect.go Outdated Show resolved Hide resolved
@martinhpedersen
Copy link
Member

Thanks 👍 I've been working on some changes on top of this, to add support for a "default bandwidth". I'll push this as a separate PR once I've had time to test it properly.

@xylo04
Copy link
Contributor Author

xylo04 commented Apr 18, 2022

Any more changes needed on this before merging?

@martinhpedersen
Copy link
Member

I guess we could merge this as is 🤔 My changes will refactor some of the web code though. I can push a draft PR so you can plan with regards to your VARA branch. Does this sounds good to you?

@martinhpedersen martinhpedersen changed the base branch from develop to connect-bandwidth April 21, 2022 19:44
@martinhpedersen
Copy link
Member

I'm merging this to a new branch, so we can continue working on it. Will add a PR that builds on this soon.

@martinhpedersen martinhpedersen merged commit 578b573 into la5nta:connect-bandwidth Apr 21, 2022
@martinhpedersen martinhpedersen mentioned this pull request Apr 21, 2022
Merged
@xylo04 xylo04 deleted the ardop-bandwidth branch June 17, 2022 00:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@martinhpedersen martinhpedersen martinhpedersen left review comments

Assignees

@xylo04 xylo04

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@xylo04 @martinhpedersen