GenericBatteryGet and GenericBatteryStatus implementation #533
Conversation
|
|
|
@JulesDommartin thank you for your contribution and it will definitely help our project! By the way please make sure to sign the CLA so that I can merge your changes straight away, |
| @@ -40,9 +40,9 @@ void parseStatusParameters() { | |||
| final ByteBuffer buffer = ByteBuffer.wrap(mParameters).order(ByteOrder.LITTLE_ENDIAN); | |||
There was a problem hiding this comment.
We don't need a byte buffer here, as the endianness will not matter as we are just dealing with single octets.
There was a problem hiding this comment.
You are right. I will remove this line and work with mParameters directly.
| if (buffer.limit() > GENERIC_BATTERY_STATUS_MANDATORY_LENGTH) { | ||
| mTimeToDischarge = buffer.get() | (buffer.get() << 8) | (buffer.get() << 16); | ||
| mTimeToCharge = buffer.get() | (buffer.get() << 8) | (buffer.get() << 16); | ||
| if (buffer.limit() >= GENERIC_BATTERY_STATUS_MANDATORY_LENGTH) { |
There was a problem hiding this comment.
This check is not required as the status will be always 8 bytes,
There was a problem hiding this comment.
Should I remove this check though ? I thought it will be useful in case of a bad implementation in the embedded software.
| mTimeToDischarge = (buffer.get() & 0xFF) | ((buffer.get() & 0xFF) << 8) | ((buffer.get() & 0xFF) << 16); | ||
| mTimeToCharge = (buffer.get() & 0xFF) | ((buffer.get() & 0xFF)<< 8) | ((buffer.get() & 0xFF) << 16); |
There was a problem hiding this comment.
Once we get rid of the bytebuffer, we could directly work with the mParameters
(mParameters[1] & 0xFF) | ((mParameters[2] & 0xFF) << 8) | (mParameters[3] & 0xFF) << 16) and so forth.
| @@ -40,9 +40,9 @@ void parseStatusParameters() { | |||
| final ByteBuffer buffer = ByteBuffer.wrap(mParameters).order(ByteOrder.LITTLE_ENDIAN); | |||
| mBatteryLevel = buffer.get(); | |||
There was a problem hiding this comment.
Once we get rid of the byte buffer we could get the battery status as follows may be mParameters[0] & 0xFF?
There was a problem hiding this comment.
I think that the & 0xFF is not needed here because the value is between 0x00 and 0x64. It can only be 0xFF if the battery level is unknown following the Mesh specifications, if I am right. But yes, like you said I will get the value from mParameters ! 😄
| mTimeToCharge = buffer.get() | (buffer.get() << 8) | (buffer.get() << 16); | ||
| if (buffer.limit() >= GENERIC_BATTERY_STATUS_MANDATORY_LENGTH) { | ||
| mTimeToDischarge = (buffer.get() & 0xFF) | ((buffer.get() & 0xFF) << 8) | ((buffer.get() & 0xFF) << 16); | ||
| mTimeToCharge = (buffer.get() & 0xFF) | ((buffer.get() & 0xFF)<< 8) | ((buffer.get() & 0xFF) << 16); | ||
| mFlags = buffer.get(); |
|
I don't understand why the CLA shows that I didn't accept them. It seems like it did work What should I do more ? Did I missed something ? |
Nevermind, I got it, my account wasn't configured properly. Now it is fine ! 😄 |
| @Override | ||
| void parseStatusParameters() { | ||
| MeshLogger.verbose(TAG, "Received generic battery status from: " + MeshAddress.formatAddress(mMessage.getSrc(), true)); | ||
| mBatteryLevel = mParameters[0]; |
There was a problem hiding this comment.
The & 0xFF is needed here to convert to an unsigned byte.
| if (mParameters.length >= GENERIC_BATTERY_STATUS_MANDATORY_LENGTH) { | ||
| mTimeToDischarge = (mParameters[1] & 0xFF) | ((mParameters[2] & 0xFF) << 8) | ((mParameters[3] & 0xFF) << 16); | ||
| mTimeToCharge = (mParameters[4] & 0xFF) | ((mParameters[5] & 0xFF) << 8) | ((mParameters[6] & 0xFF) << 16); | ||
| mFlags = mParameters[7]; |
There was a problem hiding this comment.
The & 0xFF is needed here to convert to an unsigned byte.
Hello, since I asked for this feature, I tried to implement it myself following the same code architecture and syntax as the other mesh Generic messages.
I hope it will be merged soon, and that it will help the project !