-
Notifications
You must be signed in to change notification settings - Fork 676
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
Update MQTT.cpp #1904
Update MQTT.cpp #1904
Conversation
// removing time and post_timestamp in the json seem to fix the transmission of json POSITION_APP messages to broker. (Don't know why it works but the protobuf in cleartext does not include time/timestamp either)
looks like it doesn't like the fixed32 type. time will only be included if the device has an actual GPS and timestamp needs to be enabled in the position flags. |
i'd rather prefer not to exclude these but fix the (potential) underlying problem in the json11 lib, which also lives in our github :-) |
🤖 Pull request artifacts
|
Is this a manually set fixed position? The position packet is built from flags so those flags need to be enabled for them to show up in a packet, there may not be a time if there is not a GPS. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably better to manage that these fields (and the other position flags including altitude) are optional and can be configured per device.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the field is just timestamp too now, the pos_ notation has been removed from the protobufs
pos_timestamp is just the name in the json, that should not trigger that bug. i still think it's the special datatype for these 2 fields that silently fails to convert into something JSON understands. |
Do you have a device with a GPS? Seems weird that your position packets don't have the time field. pos_timestamp should be removed but time should always be there if you have a GPS or NTP server connected |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we have two issues here, can you update this to remove pos_timestamp completely and un-comment out the time field?(this is an optional field that is seldomly going to be enabled)
The time field is potentially a separate issue related to setting fixed position that we will need to test through. It does seem like JSON11 does not love a fixed32 value of 0 for a date.
Sure, will try to work on it tomorrow. I was using a t-beam with GPS on. The other thing to consider is adding the originator device ID for the GPS data within the position protobuf intead of the envelope. It seems that if you have multiple devices sending positions via LORA to an MQTT connected device -- all those position packets get stripped out and re-eneveloped as coming from the MQTT connected device before being sent to the broker as JSON. This makes it impossible to do plotting/geofencing for multiple units on a mesh using MQTT data unless they all individually connect via MQTT and originate their own position data. |
Sorry, but un-commenting the time, getting rid of the post-timestamp and updating everything else to the current repository 2.03 still results in no JSON going to the MQTT broker. Two t-beams with gps enabled on the mesh, one with mqtt and network enabled. 02:19:43 916 [Router] Incoming message was filtered 0x55c7312c ---------------- but later on in the log "time" does show up in the log but still not in json at the broker 02:27:26 1380 [PositionModule] Update DB node 0x55c7312c, rx_time=1668133646 |
Deleting the time field is not the right solution, we can merge it and test if just the pos_timestamp field that is never in the packet is removed. |
// removing time and post_timestamp in the json seem to fix the transmission of json POSITION_APP messages to broker. (Don't know why it works but the protobuf in cleartext does not include time/timestamp either)