Skip to content

ninafw: allow custom config #224

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from
Open

ninafw: allow custom config #224

wants to merge 1 commit into from

Conversation

bgould
Copy link
Member

@bgould bgould commented Jan 7, 2024

Refactoring ninafw adapter to allow for settings and pin configurations separate from the machine package.

@bgould bgould changed the base branch from release to dev January 7, 2024 00:29
Copy link
Member

@deadprogram deadprogram left a comment

Choose a reason for hiding this comment

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

@bgould bgould changed the base branch from dev to ninafw-peripheral January 16, 2024 00:13
@bgould bgould changed the title Draft: Ninafw custom config Ninafw custom config Jan 16, 2024
@bgould bgould force-pushed the ninafw-custom-config branch from b65e00d to 2838920 Compare January 16, 2024 03:36
@bgould bgould changed the base branch from ninafw-peripheral to dev January 16, 2024 03:55
@bgould bgould force-pushed the ninafw-custom-config branch from 2838920 to 731e738 Compare January 16, 2024 03:59
@bgould bgould changed the title Ninafw custom config ninafw: allow custom config Jan 18, 2024
@bgould bgould force-pushed the ninafw-custom-config branch from 731e738 to f100ba6 Compare January 18, 2024 01:17
@bgould
Copy link
Member Author

bgould commented Jan 18, 2024

@deadprogram This is rebased against the latest from the dev branch and ready for feedback/merging

@@ -11,6 +11,29 @@ import (

const maxConnections = 1

// NINAConfig encapsulates the hardware options for the NINA firmware
type NINAConfig struct {
Copy link
Member

Choose a reason for hiding this comment

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

I suggest name this HCIConfig so it can be more easily used for non-NINA HCI controllers.

@@ -0,0 +1,22 @@
//go:build ninafw && ninafw_featherwing_init
Copy link
Member

Choose a reason for hiding this comment

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

I think ninafw_featherwing_init can just be featherwing.

@@ -0,0 +1,22 @@
//go:build ninafw && ninafw_machine_init
Copy link
Member

Choose a reason for hiding this comment

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

How about name ninafw_machine_init instead to ninafw_onboard?

@deadprogram
Copy link
Member

@bgould I made a few comments. What I have in mind is to create a adapter_hci.go for pure HCI controllers that do not use the NINAFW just to explain in part why I made those comments.

@aykevl
Copy link
Member

aykevl commented Feb 17, 2024

What's the status of this PR? This looks like a good idea to me (and I agree with the suggestions of @deadprogram).

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants