Skip to content
This repository was archived by the owner on Aug 18, 2020. It is now read-only.

LoRa support #48

Merged
merged 9 commits into from
Dec 16, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 20 additions & 11 deletions src/ArduinoCloudProperty.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,7 @@
#endif

static unsigned long getTimestamp() {
#ifdef ARDUINO_ARCH_SAMD
Copy link
Contributor

Choose a reason for hiding this comment

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

Why was the content of this function deleted?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Now the code is restored.

Hi @mirkokurt 👋 I have a very hard time understanding the changes done within this PR. Please outline how the CBOR protocol was extended/changed and why.

Also I've got a esspecially a big problem with test code were the comment was changed but the actual byte array was not changed (here) with a commit message saying Fix unit tests. Why were those tests fixed? I'm looking at magic numbers without explanation in the test code. This is unacceptable.

When writing any code please keep in mind that code is written for your fellow programmers first and for the machine second.

Hi @lxrobotics, I added many comments in the code to help programmers understanding how the library encode and decode the cbor messages when the option of using "light payloads" has been activated. I also updated the PR description to better explain the purpose of the implementation.

Regarding the commit Fix unit tests, there are two kind of fixes in the commit:
the first kind is a logic fix:
eg:
-thing.addPropertyReal(color_test, "test", Permission::ReadWrite);
+thing.addPropertyReal(color_test, "test", Permission::ReadWrite, 1);

the addPropertyReal method was invoked without an identifier assigned to the property. So the test was not well written.

The second kind is a fix in the comment that was previously wrong in respect of the byte array.
eg:
-/* [{0: "test", 4: false}] = 81 A2 00 01 04 F4 /
+/
[{0: 1, 4: false}] = 81 A2 00 01 04 F4 */

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why was the content of this function deleted?

By mistake. Now the code is restored.

return rtc.getEpoch();
#else
#pragma message "No RTC available on this architecture - ArduinoIoTCloud will not keep track of local change timestamps ."
return 0;
#endif
return 0;
}

/******************************************************************************
Expand All @@ -47,6 +42,7 @@ ArduinoCloudProperty::ArduinoCloudProperty()
_update_interval_millis(0),
_last_local_change_timestamp(0),
_last_cloud_change_timestamp(0),
_position(0),
_map_data_list(nullptr) {
}

Expand Down Expand Up @@ -151,14 +147,23 @@ void ArduinoCloudProperty::appendAttributeReal(String value, String attributeNam
}

void ArduinoCloudProperty::appendAttributeName(String attributeName, std::function<void (CborEncoder& mapEncoder)>appendValue, CborEncoder *encoder) {

CborEncoder mapEncoder;
cbor_encoder_create_map(encoder, &mapEncoder, 2);
cbor_encode_int(&mapEncoder, static_cast<int>(CborIntegerMapKey::Name));
String completeName = _name;
if (attributeName != "") {
completeName += ":" + attributeName;
}
cbor_encode_text_stringz(&mapEncoder, completeName.c_str());

#ifdef SerialLoRa
Serial.println("I'm a lora device!");
cbor_encode_int(&mapEncoder, _position);
#else
Serial.println("I'm NOT a lora device!");
String completeName = _name;
if (attributeName != "") {
completeName += ":" + attributeName;
}
cbor_encode_text_stringz(&mapEncoder, completeName.c_str());
#endif

appendValue(mapEncoder);
cbor_encoder_close_container(encoder, &mapEncoder);
}
Expand Down Expand Up @@ -233,3 +238,7 @@ unsigned long ArduinoCloudProperty::getLastCloudChangeTimestamp() {
unsigned long ArduinoCloudProperty::getLastLocalChangeTimestamp() {
return _last_local_change_timestamp;
}

void ArduinoCloudProperty::setPosition(int pos) {
_position = pos;
}
10 changes: 10 additions & 0 deletions src/ArduinoCloudProperty.h
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ class CborMapData {
MapEntry<double> base_time;
MapEntry<String> name;
MapEntry<String> attribute_name;
MapEntry<int> property_position;
MapEntry<float> val;
MapEntry<String> str_val;
MapEntry<bool> bool_val;
Expand Down Expand Up @@ -138,6 +139,9 @@ class ArduinoCloudProperty {
inline String name() const {
return _name;
}
inline int position() const {
return _position;
}
inline bool isReadableByCloud() const {
return (_permission == Permission::Read) || (_permission == Permission::ReadWrite);
}
Expand All @@ -152,6 +156,7 @@ class ArduinoCloudProperty {
void setLastLocalChangeTimestamp(unsigned long localChangeTime);
unsigned long getLastCloudChangeTimestamp();
unsigned long getLastLocalChangeTimestamp();
void setPosition(int pos);

void updateLocalTimestamp();
void append(CborEncoder * encoder);
Expand All @@ -173,6 +178,9 @@ class ArduinoCloudProperty {
virtual void fromLocalToCloud() = 0;
virtual void appendAttributesToCloudReal(CborEncoder *encoder) = 0;
virtual void setAttributesFromCloud() = 0;
virtual String getAttributeNameByPosition(int position) {
return "";
};
virtual bool isPrimitive() {
return false;
};
Expand All @@ -197,6 +205,8 @@ class ArduinoCloudProperty {
unsigned long _last_local_change_timestamp;
unsigned long _last_cloud_change_timestamp;
LinkedList<CborMapData *> * _map_data_list;
/* Store the position of the property in the array list */
int _position;
};

/******************************************************************************
Expand Down
59 changes: 58 additions & 1 deletion src/ArduinoCloudThing.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ void PrintFreeRam(void) {

ArduinoCloudThing::ArduinoCloudThing() :
_numPrimitivesProperties(0),
_numProperties(0),
_isSyncMessage(false),
_currentPropertyName(""),
_currentPropertyBaseTime(0),
Expand Down Expand Up @@ -92,9 +93,11 @@ ArduinoCloudProperty& ArduinoCloudThing::addPropertyReal(ArduinoCloudProperty &
if (property.isPrimitive()) {
_numPrimitivesProperties++;
}
_numProperties++;
addProperty(&property);
return (property);
}

}

void ArduinoCloudThing::decode(uint8_t const * const payload, size_t const length, bool isSyncMessage) {
Expand Down Expand Up @@ -180,6 +183,19 @@ ArduinoCloudProperty * ArduinoCloudThing::getProperty(String const & name) {
return NULL;
}

ArduinoCloudProperty * ArduinoCloudThing::getProperty(int const & pos) {
// The alternative to evaluate is simply:
// return _property_list.get(pos-1);
// to be verified if the position in the list is ALWAYS the id of the property
for (int i = 0; i < _property_list.size(); i++) {
ArduinoCloudProperty * p = _property_list.get(i);
if (p->position() == pos) {
return p;
}
}
return NULL;
}

// this function updates the timestamps on the primitive properties that have been modified locally since last cloud synchronization
void ArduinoCloudThing::updateTimestampOnLocallyChangedProperties() {
if (_numPrimitivesProperties == 0) {
Expand Down Expand Up @@ -326,8 +342,32 @@ ArduinoCloudThing::MapParserState ArduinoCloudThing::handle_Name(CborValue * val
map_data->attribute_name.set(attribute_name);
next_state = MapParserState::MapKey;
}
} else if (cbor_value_is_integer(value_iter)) {
int val = 0;
if (cbor_value_get_int(value_iter, &val) == CborNoError) {
Serial.print("Position to search is: ");
Serial.println(val);
String name = getPropertyNameByPosition(val);
Serial.print("Name of the property: ");
Serial.println(name);
map_data->name.set(name);
int colonPos = name.indexOf(":");
String attribute_name = "";
if (colonPos != -1) {
attribute_name = name.substring(colonPos + 1);
}
Serial.print("Name of the attribute: ");
Serial.println(name);
map_data->attribute_name.set(attribute_name);

if (cbor_value_advance(value_iter) == CborNoError) {
next_state = MapParserState::MapKey;
}
}
}



return next_state;
}

Expand Down Expand Up @@ -404,7 +444,6 @@ ArduinoCloudThing::MapParserState ArduinoCloudThing::handle_LeaveMap(CborValue *
}

if (_currentPropertyName != "" && propertyName != _currentPropertyName) {

/* Update the property containers depending on the parsed data */
updateProperty(_currentPropertyName, _currentPropertyBaseTime + _currentPropertyTime);
/* Reset current property data */
Expand Down Expand Up @@ -461,6 +500,24 @@ void ArduinoCloudThing::updateProperty(String propertyName, unsigned long cloudC
}
}
}

String ArduinoCloudThing::getPropertyNameByPosition(int propertyPosition) {
if(propertyPosition>255) {
Serial.print("Position is: ");
Serial.println(propertyPosition & 255);
Serial.print("Sub-Position is: ");
Serial.println(propertyPosition >> 8);
ArduinoCloudProperty* property = getProperty(propertyPosition & 255);
String attributeName = property->getAttributeNameByPosition(propertyPosition >> 8);
return property->name() + ":" + attributeName;
} else {
Serial.print("Position is: ");
Serial.println(propertyPosition & 255);
ArduinoCloudProperty* property = getProperty(propertyPosition);
return property->name();
}
}

bool ArduinoCloudThing::ifNumericConvertToDouble(CborValue * value_iter, double * numeric_val) {

if (cbor_value_is_integer(value_iter)) {
Expand Down
6 changes: 5 additions & 1 deletion src/ArduinoCloudThing.h
Original file line number Diff line number Diff line change
Expand Up @@ -89,11 +89,13 @@ class ArduinoCloudThing {
int appendChangedProperties(CborEncoder * arrayEncoder);
void updateTimestampOnLocallyChangedProperties();
void updateProperty(String propertyName, unsigned long cloudChangeEventTime);
String getPropertyNameByPosition(int propertyPosition);

private:
LinkedList<ArduinoCloudProperty *> _property_list;
/* Keep track of the number of primitive properties in the Thing. If 0 it allows the early exit in updateTimestampOnLocallyChangedProperties() */
int _numPrimitivesProperties;
int _numProperties;
/* Indicates the if the message received to be decoded is a response to the getLastValues inquiry */
bool _isSyncMessage;
/* List of map data that will hold all the attributes of a property */
Expand Down Expand Up @@ -135,11 +137,13 @@ class ArduinoCloudThing {

static bool ifNumericConvertToDouble(CborValue * value_iter, double * numeric_val);
static double convertCborHalfFloatToDouble(uint16_t const half_val);
void freeMapDataList(LinkedList<CborMapData *> *map_data_list);
void freeMapDataList(LinkedList<CborMapData *> * map_data_list);
inline void addProperty(ArduinoCloudProperty * property_obj) {
property_obj->setPosition(_numProperties);
_property_list.add(property_obj);
}
ArduinoCloudProperty * getProperty(String const & name);
ArduinoCloudProperty * getProperty(int const & position);

};

Expand Down
11 changes: 11 additions & 0 deletions src/types/CloudColor.h
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,17 @@ class CloudColor : public ArduinoCloudProperty {
setAttribute(_cloud_value.sat);
setAttribute(_cloud_value.bri);
}
virtual String getAttributeNameByPosition(int position) {
if(position == 1) {
return "hue";
} else if (position == 2) {
return "sat";
} else if (position == 3) {
return "bri";
} else {
return "";
}
};
};

#endif /* CLOUDCOLOR_H_ */
9 changes: 9 additions & 0 deletions src/types/CloudLocation.h
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,15 @@ class CloudLocation : public ArduinoCloudProperty {
setAttribute(_cloud_value.lat);
setAttribute(_cloud_value.lon);
}
virtual String getAttributeNameByPosition(int position) {
if(position == 1) {
return "lat";
} else if (position == 2) {
return "lon";
} else {
return "";
}
};
};

#endif /* CLOUDLOCATION_H_ */