inject_tlm performed by microservices #614
Conversation
…router commanding
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #614 +/- ##
==========================================
+ Coverage 74.48% 74.75% +0.27%
==========================================
Files 457 458 +1
Lines 28270 28359 +89
Branches 588 588
==========================================
+ Hits 21056 21200 +144
+ Misses 7121 7066 -55
Partials 93 93
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 3 files with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
openc3/lib/openc3/api/tlm_api.rb
Outdated
| else | ||
| packet.received_count = 1 | ||
| DecomTopic.inject_tlm(target_name, packet_name, item_hash, type: type, scope: scope) |
There was a problem hiding this comment.
Most targets have interfaces but if not we use Decom which is always deployed for each packet. What does the use of the Interface get you over Decom?
There was a problem hiding this comment.
Its a matter of who is keeping track of received_count
| read_index += 1 | ||
| write_index += 1 | ||
| end | ||
| end |
There was a problem hiding this comment.
So the bug was not building up the list according to their type. Previously it would have gone through all whether they were :READ or :WRITE.
There was a problem hiding this comment.
The real bug was not command WRITE protocols if READ_WRITE was specified. READ_WRITE is suppose to be effectively all for protocols
| decom_packet(topic, msg_id, msg_hash, redis) | ||
| if topic =~ /__DECOMINTERFACE/ | ||
| if msg_hash.key?('inject_tlm') | ||
| handle_inject_tlm(msg_hash['inject_tlm']) |
There was a problem hiding this comment.
Seems like __DECOMINTERFACE__ is a dedicated topic for inject_tlm. Is there a reason to check the msg_hash for inject_tlm? Is this going to be used for something else?
There was a problem hiding this comment.
Just inject_tlm for now. Might be other things in the future.
| @@ -164,7 +171,7 @@ def run | |||
| if target_name | |||
| command = System.commands.identify(cmd_buffer, [target_name]) | |||
| else | |||
| command = System.commands.identify(cmd_buffer, @cmd_target_names) | |||
| command = System.commands.identify(cmd_buffer, @interface.cmd_target_names) | |||
There was a problem hiding this comment.
This is just a straight up bug. How have we never hit this before? And therefore is it even necessary?
There was a problem hiding this comment.
This code is needed if not using the PREIDENTIFIED interface
| packet = identified_packet if identified_packet | ||
| end | ||
|
|
||
| begin | ||
| RouterTopic.route_command(packet, @cmd_target_names, scope: @scope) | ||
| RouterTopic.route_command(packet, @interface.cmd_target_names, scope: @scope) |
There was a problem hiding this comment.
So this was just getting rescued and logging errors.
| RouterStatusModel.set(@interface.as_json(:allow_nan => true), scope: @scope) | ||
| if !packet.identified? | ||
| # Need to identify so we can find the target | ||
| identified_packet = System.commands.identify(packet.buffer(false), @cmd_target_names) | ||
| identified_packet = System.commands.identify(packet.buffer(false), @interface.cmd_target_names) |
There was a problem hiding this comment.
I guess all packets are identified because otherwise this would have crashed the router.
There was a problem hiding this comment.
Yes, when we are using the PREIDENTIFIED protocol
| require 'openc3/topics/topic' | ||
|
|
||
| module OpenC3 | ||
| class DecomTopic < Topic |
There was a problem hiding this comment.
We have CommandDecomTopic and TelemetryDecomTopic ... should this be InterfaceDecomTopic?
There was a problem hiding this comment.
Good point. I should just put this in TelemetryDecomTopic
ryanmelt
changed the title
also repair interface_cmd, protocol_cmd, and router commanding
closes #518