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

[Golden maple] Air_Temperature_Humidity - 10310 #82

Conversation

peaceblues
Copy link

Update to Apache 2.0 License

@seanmcilroy29 seanmcilroy29 changed the title Golden maple data [Golden maple] Air_Temperature_Humidity - 10310 Mar 12, 2019
@seanmcilroy29
Copy link

seanmcilroy29 commented Mar 12, 2019

Observations

  • OMA copyright licence is still visible - Needs to be removed
  • Object version requires to be inserted

@peaceblues
Copy link
Author

@seanmcilroy29 I‘ve updated the .xml file, please review, thanks.

@peaceblues
Copy link
Author

@jpradocueva updated, plz review.

@jpradocueva
Copy link
Member

@peaceblues the Object is ready for IPSO Working Group discussion, tomorrow Thursday 14th. Please keep an eye in case that further comments are received. You may have to provide another update. If the group agrees with the content it will be merged into the "Golden-maple-data" branch and the staff will move it to publication.

OMA is aligning the unit symbol to SenML registry: https://www.iana.org/assignments/senml/senml.xhtml
In this case, the temperature in Celsius is should represented as "Cel" rather than "C".
@peaceblues
Copy link
Author

@jpradocueva, hello, the object has passed the review and published , right?

@seanmcilroy29
Copy link

seanmcilroy29 commented Mar 14, 2019 via email

@peaceblues
Copy link
Author

@jpradocueva @seanmcilroy29 Any progress on this?

@seanmcilroy29
Copy link

A few questions were raised during the working group review of the Object, so the Chair has requested a 6-day review period in order for members to submit comments/questions for you to answer.

Copy link

@mcocak mcocak left a comment

Choose a reason for hiding this comment

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

I've made some comments about some smaller issues I've found in this object registration.

However, I'm not sure about the need for this object in general. Objects generally self-contain one functionality such as Temperature, Humidity and Voltage objects already separately exist. This object uses 3 different resources for these 3 already existing objects. At the end, this object does not offer any new functionality in my opinion. It is just a shortcut to use existing objects, by bypassing the principle of "one object one functionality".

limitations under the License.-->
<LWM2M xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="http://www.openmobilealliance.org/tech/profiles/LWM2M-v1_1.xsd">
<Object ObjectType="MODefinition">
<Name>Air_Temperature_Humidity</Name>
Copy link

Choose a reason for hiding this comment

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

It should be full sentences here i.e. Air Temperature Humidity

<LWM2MVersion>1.1</LWM2MVersion>
<ObjectVersion>1.0</ObjectVersion>
<MultipleInstances>Multiple</MultipleInstances>
<Mandatory>Mandatory</Mandatory>
Copy link

Choose a reason for hiding this comment

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

This object cannot be mandatory! Change to Optional

<Mandatory>Mandatory</Mandatory>
<Resources>
<Item ID="1">
<Name>air temperature</Name>
Copy link

Choose a reason for hiding this comment

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

Capitalize initials: Air Temperature

<MultipleInstances>Single</MultipleInstances>
<Mandatory>Mandatory</Mandatory>
<Type>Float</Type>
<RangeEnumeration>-100~100</RangeEnumeration>
Copy link

Choose a reason for hiding this comment

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

Range should be stated as -100..100

<Description><![CDATA[Air temperature]]></Description>
</Item>
<Item ID="2">
<Name>air humidity</Name>
Copy link

Choose a reason for hiding this comment

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

Capitalize initials: Air Humidity

<Description><![CDATA[Air humidity]]></Description>
</Item>
<Item ID="3">
<Name>voltage</Name>
Copy link

Choose a reason for hiding this comment

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

Capitalize initial: Voltage

<MultipleInstances>Single</MultipleInstances>
<Mandatory>Optional</Mandatory>
<Type>Float</Type>
<RangeEnumeration>0~24</RangeEnumeration>
Copy link

Choose a reason for hiding this comment

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

Range should be stated as 0..24

<Description><![CDATA[Voltage]]></Description>
</Item>
<Item ID="4">
<Name>report interval</Name>
Copy link

Choose a reason for hiding this comment

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

Capitalize initials: Report Interval

</Item>
<Item ID="4">
<Name>report interval</Name>
<Operations>W</Operations>
Copy link

@mcocak mcocak Mar 18, 2019

Choose a reason for hiding this comment

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

Is this resource really meant to have only W access? Why not RW since it is probably easier to manage?

<MultipleInstances>Single</MultipleInstances>
<Mandatory>Optional</Mandatory>
<Type>Integer</Type>
<RangeEnumeration>1~25</RangeEnumeration>
Copy link

Choose a reason for hiding this comment

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

Range should be stated as 1..25

@ScottTaftPotter
Copy link
Contributor

The units on resource 2, air humidity, should probably be %RH per the new SenML units we are standardizing.

@ScottTaftPotter
Copy link
Contributor

I've made some comments about some smaller issues I've found in this object registration.

However, I'm not sure about the need for this object in general. Objects generally self-contain one functionality such as Temperature, Humidity and Voltage objects already separately exist. This object uses 3 different resources for these 3 already existing objects. At the end, this object does not offer any new functionality in my opinion. It is just a shortcut to use existing objects, by bypassing the principle of "one object one functionality".

We would like the submitter to first comment on this suggestion to consider the use of existing objects.

Company Name: Golden Maple Data
Remove global URL
@seanmcilroy29
Copy link

@peaceblues - Please note the Working Group are waiting for you to respond to their submitted comments prior to having the Object Approved

@jpradocueva
Copy link
Member

@peaceblues please review the latest comments. If we haven't receive any reply by May 16th we will have to close the PR and the registration. Thanks

@seanmcilroy29
Copy link

@peaceblues - Please note the Working Group will be holding a conf call this Thursday [16 MAY]. If no reply has been received the PR and Registration for Air_Temperature_Humidity will be closed.

@seanmcilroy29
Copy link

@peaceblues - Please note the Working Group will be holding a conf call this Thursday [20 Jun]. If no reply has been received the PR and Registration for Air_Temperature_Humidity will be closed

@jpradocueva
Copy link
Member

This PR is closed as the submitter has not followed up on the comments from the Group.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants