Support custom fragments in alarm creation#1699
Support custom fragments in alarm creation#1699PradeepKiruvale merged 1 commit intothin-edge:mainfrom
Conversation
|
@PradeepKiruvale Look ok from a non-rust pov. I will let @albinsuresh (or some other rust enabled person) to formally approve to prevent it from being merged prematurely. |
crates/core/tedge_api/src/alarm.rs
Outdated
| pub time: Option<Timestamp>, | ||
|
|
||
| #[serde(flatten)] | ||
| pub extras: HashMap<String, Value>, |
There was a problem hiding this comment.
Why is this named extras? We use the term fragment in the documentation.
There was a problem hiding this comment.
I agree, we can rename it as fragments. But it will also contain the child device info as part of the message. Can this child device info also be called a fragment? I am not sure. So, I kept it in line with the events.
There was a problem hiding this comment.
If the child-device-id is stored as an entry in this extras map, what's the content of the source ? I though it was the id of the child device sending the alarm?
There was a problem hiding this comment.
I though it was the id of the child device sending the alarm?
Yes it is. Not just for child devices, but even for the parent device.
There was a problem hiding this comment.
I renamed extras as fragments
| anyhow = "1.0" | ||
| assert-json-diff = "2.0" | ||
| assert_matches = "1.5" | ||
| maplit = "1.0.2" |
There was a problem hiding this comment.
This is fine as a dev dependency. But just FYI: you could statically create HashMap using the from method as follows:
let solar_distance = HashMap::from([
("Mercury", 0.4),
("Venus", 0.7),
("Earth", 1.0),
("Mars", 1.5),
]);
crates/core/tedge_api/src/alarm.rs
Outdated
| Some(serde_json::from_str(mqtt_payload)?) | ||
| }; | ||
|
|
||
| // The 4th part of the topic name is the event source - if any |
There was a problem hiding this comment.
| // The 4th part of the topic name is the event source - if any | |
| // The 4th part of the topic name is the alarm source - if any |
| ); | ||
| output_messages.push(Message::new(&c8y_alarm_topic, smartrest_alarm)); | ||
| } else { | ||
| dbg!(&c8y_alarm); |
There was a problem hiding this comment.
Don't forget to remove this at the end.
crates/core/c8y_api/src/json_c8y.rs
Outdated
| fn update_the_source( | ||
| source_name: &str, | ||
| ) -> Result<Option<HashMap<String, String>>, SMCumulocityMapperError> { |
There was a problem hiding this comment.
The name and the type signature of this function hardly convey its purpose.
I would simply return a hash.
| fn update_the_source( | |
| source_name: &str, | |
| ) -> Result<Option<HashMap<String, String>>, SMCumulocityMapperError> { | |
| fn make_c8y_source_fragment( | |
| source_name: &str, | |
| ) -> HashMap<String, String> { |
crates/core/c8y_api/src/json_c8y.rs
Outdated
| #[serde(skip_serializing_if = "Option::is_none")] | ||
| #[serde(rename = "externalSource")] | ||
| pub source: Option<HashMap<String, String>>, |
There was a problem hiding this comment.
I would consider to remove the Option type, using an empty map instead of None.
There was a problem hiding this comment.
I replaced the hash map with a SourceInfo struct for the source.
crates/core/c8y_api/src/json_c8y.rs
Outdated
| #[serde(skip_serializing_if = "Option::is_none")] | ||
| pub source: Option<C8yManagedObject>, | ||
| #[serde(rename = "externalSource")] | ||
| pub source: Option<HashMap<String, String>>, |
There was a problem hiding this comment.
Instead of using a generic Hashmap here, I'd use a struct that replicates externalSource JSON structure:
"externalSource":{
"externalId":"<child-device-id>",
"type":"c8y_Serial"
}|
@PradeepKiruvale I found the reason why the alarm is not visible in the Cumulocity UI, the I just tested with your version here, and below is the json that I saw was being pushed to the {
"externalSource": {
"externalId": "TST-extend_pure_loop_child1",
"type": "c8y_Serial"
},
"severity": "critical", // <<<<<<<<<<<<<<<<< needs to be UPPERCASE
"type": "myCustomAlarmType",
"time": "2023-02-01T21:08:14.800229538Z",
"text": "Some test alarm",
"someOtherCustomFragment": {
"nested": {
"value": "extra info"
}
}
} |
Good catch, but I remember testing with all CAPS. But still, it was not working then, Now it works, strange. Anyways I have updated the code to address this issue. |
a6b539a to
d40ed80
Compare
Signed-off-by: Pradeep Kumar K J <pradeepkumar.kj@softwareag.com>
e244f63 to
d09703f
Compare
Signed-off-by: Reuben Miller <reuben.d.miller@gmail.com>
Signed-off-by: Reuben Miller <reuben.d.miller@gmail.com>
Signed-off-by: Reuben Miller <reuben.d.miller@gmail.com>
Signed-off-by: Reuben Miller <reuben.d.miller@gmail.com>
Signed-off-by: Reuben Miller <reuben.d.miller@gmail.com>
Signed-off-by: Reuben Miller <reuben.d.miller@gmail.com>
Signed-off-by: Reuben Miller <reuben.d.miller@gmail.com>
Signed-off-by: Reuben Miller <reuben.d.miller@gmail.com>
Signed-off-by: Reuben Miller <reuben.d.miller@gmail.com>
Signed-off-by: Reuben Miller <reuben.d.miller@gmail.com>
Signed-off-by: Reuben Miller <reuben.d.miller@gmail.com>
Signed-off-by: Pradeep Kumar K J pradeepkumar.kj@softwareag.com
Proposed changes
Create an alarm with custom fragments
Supports for both
thin-edgedevices as well as for thechilddevices.For example, if an alarm message is published with a custom fragment as below a child device it must create an alarm with the custom message.
Types of changes
Paste Link to the issue
#1682
Checklist
cargo fmtas mentioned in CODING_GUIDELINEScargo clippyas mentioned in CODING_GUIDELINESFurther comments