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

Create connected-network-type-subscriptions.yaml #171

Merged
merged 13 commits into from
Jan 15, 2025

Conversation

VijayKesharwani
Copy link
Contributor

What type of PR is this?

Add one of the following kinds:

  • bug
  • correction
  • enhancement/feature
  • cleanup
  • documentation
  • subproject management
  • tests

What this PR does / why we need it:

(#143)

Which issue(s) this PR fixes:

(#143)

Fixes #

Special notes for reviewers:

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 #158 to the newly created https://github.com/camaraproject/DeviceQualityIndicator, do you agree?

@akoshunyadi
Copy link
Collaborator

@camaraproject/device-status_codeowners I guess this can be moved together with #158 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.

@sachinvodafone
Copy link
Collaborator

It is mentioned under quality indicator "Get insights about the network status of a defined mobile device." so I am not sure if it will have some other inputs other than mobile device.

@akoshunyadi
Copy link
Collaborator

camaraproject/APIBacklog#19

sachinvodafone
sachinvodafone previously approved these changes Oct 17, 2024
@akoshunyadi
Copy link
Collaborator

@VijayKesharwani Let's keep this subscription API in sync with the non-subscription one in #158. There there was a change in the enums. As soon as we have an approval there this PR should be updated. Thanks.

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.

Thanks !
In order to mix ourselves could you add in the yaml the commonalities reference.

In this case add a line btw 61 & 62 to add
x-camara-commonalities: 0.4.0

cc: @akoshunyadi

description: |
network-type-changed - Event triggered when the connected network type of the device changes.
enum:
- org.camaraproject.connected-network-type-subscriptions.v0.network-type-changed
Copy link
Collaborator

Choose a reason for hiding this comment

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

We need the subscription-ends event here and the corresponding description in info.description

example:
status: 429
code: TOO_MANY_REQUESTS
message: "Endpoint does not support as many requests per time slot"
Copy link
Collaborator

Choose a reason for hiding this comment

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

It seems, message under the example is not the same as we defined in the CAMARA_common.yaml. Can we keep different description other than mentioned there or would it be in synch.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Error messages are always examples. Implementations are anyway free to define their own error messages, irrespective of what is documented in the OAS definition. So individual sub-projects are also free to use different error message examples if it better clarifies a particular error for a particular API.

@akoshunyadi akoshunyadi merged commit 66d1c12 into camaraproject:main Jan 15, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants