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

Create connected-network-type.yaml #158

Merged
merged 10 commits into from
Dec 18, 2024
Merged

Create connected-network-type.yaml #158

merged 10 commits into from
Dec 18, 2024

Conversation

gmuratk
Copy link
Contributor

@gmuratk gmuratk commented Jun 7, 2024

What type of PR is this?

Add one of the following kinds:

  • enhancement/feature

What this PR does / why we need it:

#143 requests this enhancement/feature as new API under Device Status.

Which issue(s) this PR fixes:

Fixes #143

Special notes for reviewers:

Proposal doesn't include the subscriptions portion.

Changelog input

 release-note

Additional documentation

This section can be blank.

docs

@maxl2287
Copy link
Contributor

@camaraproject/device-status_codeowners I guess this can be moved together with #171 to the newly created https://github.com/camaraproject/DeviceQualityIndicator, do you agree?

@maxl2287 maxl2287 mentioned this pull request Sep 17, 2024
Open
@akoshunyadi
Copy link
Collaborator

@camaraproject/device-status_codeowners I guess this can be moved together with #171 to the newly created https://github.com/camaraproject/DeviceQualityIndicator, do you agree?

I think the quality indicator is something different. It will need different inputs, e.g. the network type, to provide a network quality indication.

@Kevsy Kevsy mentioned this pull request Sep 26, 2024
Closed
Kevsy added a commit to Kevsy/APIBacklog that referenced this pull request Sep 27, 2024
@Kevsy
Update APIScopeEnhancement_Device_Status_network_info.md
57d3a32 
Amended per API Backlog/Device Status discussion (ref Device Status [PR 158](camaraproject/DeviceStatus#158)
@akoshunyadi akoshunyadi marked this pull request as ready for review October 30, 2024 15:49
@gmuratk
Copy link
Contributor Author

gmuratk commented Nov 5, 2024

Thank you for all the comments over the past months. Since 'Draft' label is removed, we'll provide responses/edits soon.
cc: @murthygorty @RamTMO

@akoshunyadi akoshunyadi mentioned this pull request Nov 18, 2024
Open
@gmuratk
Update connected-network-type.yaml
a0e8804 
Updates:
- Use of UNKNOWN for undetermined connection [technology]
- Simplified list of connection technologies
- Updates to Examples and API documentation in line with above points
bigludo7
bigludo7 reviewed Nov 20, 2024
View reviewed changes
Copy link
Collaborator

@bigludo7 bigludo7 left a comment

Choose a reason for hiding this comment

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

One typo in the tag

code/API_definitions/connected-network-type.yaml Outdated Show resolved Hide resolved
@gmuratk
Update connected-network-type.yaml
5d94fb0 
Update v0 - vwip
Corrected tag with matching strings
@gmuratk
Update connected-network-type.yaml
93e3c7d 
changed api-name to connected-network-type
changed path to /retrieve only
added to description of /retrieve method
changed schema label from RequestConnectedNetworkType to ConnectedNetworkTypeRequest
@eric-murray
Copy link
Collaborator

@gmuratk
If you had time, you could add the modified error response schema in CAMARA_common.yaml into the PR. But as this will be a WIP, we can add that later.

sachinvodafone
sachinvodafone previously approved these changes Dec 12, 2024
View reviewed changes
akoshunyadi
akoshunyadi previously approved these changes Dec 12, 2024
View reviewed changes
@akoshunyadi
Copy link
Collaborator

@gmuratk If you had time, you could add the modified error response schema in CAMARA_common.yaml into the PR. But as this will be a WIP, we can add that later.

I would suggest to merge this PR without the adapted error codes and work on those in a separate PR, for both APIs.

@eric-murray
Copy link
Collaborator

I would suggest to merge this PR without the adapted error codes and work on those in a separate PR, for both APIs.

That's fine, but we should remove that device is required before merging

@gmuratk gmuratk dismissed stale reviews from akoshunyadi and sachinvodafone via 9052723 December 17, 2024 15:15
bigludo7
bigludo7 approved these changes Dec 18, 2024
View reviewed changes
Copy link
Collaborator

@bigludo7 bigludo7 left a comment

Choose a reason for hiding this comment

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

LGTM

@akoshunyadi akoshunyadi merged commit a0f15c5 into camaraproject:main Dec 18, 2024
2 checks passed
@murthygorty
Copy link

woohoo :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@QaunainM QaunainM QaunainM left review comments

@Kevsy Kevsy Kevsy left review comments

@maxl2287 maxl2287 maxl2287 requested changes

@sachinvodafone sachinvodafone sachinvodafone approved these changes

@eric-murray eric-murray eric-murray approved these changes

@akoshunyadi akoshunyadi akoshunyadi approved these changes

@bigludo7 bigludo7 bigludo7 approved these changes

Assignees
No one assigned
Labels
Spring25
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Extend: Device Status Connectivity with Standard
9 participants
@gmuratk @maxl2287 @akoshunyadi @eric-murray @murthygorty @Kevsy @bigludo7 @QaunainM @sachinvodafone