Anonymous | Login | Signup for a new account | 2021-01-19 23:24 UTC | ![]() |
Main | My View | View Issues | Roadmap | Repositories |
View Issue Details [ Jump to Notes ] | [ Print ] | ||||||||||
ID | Project | Category | View Status | Date Submitted | Last Update | ||||||
0000631 | EMStudio | FreeEMS Plugin | public | 2012-07-28 18:32 | 2013-01-29 13:27 | ||||||
Reporter | Fred | ||||||||||
Assigned To | malcom2073 | ||||||||||
Priority | normal | Severity | minor | Reproducibility | always | ||||||
Status | assigned | Resolution | fixed | ||||||||
Platform | All | OS | All | OS Version | All | ||||||
Product Version | |||||||||||
Target Version | 0.0.1 | Fixed in Version | |||||||||
Summary | 0000631: Reject interrogation of firmware with invalid location ID meta data | ||||||||||
Description | Reject interrogation of firmware with location IDs that have different lengths to defined location IDs in the meta data file or protocol/interface definition. Undefined locations may exist and be assigned to a hex editor. Defined locations should contain sufficient meta data to determine their length and render their contents in a human readable way. | ||||||||||
Additional Information | Special types such as 2D and 3D tables should be verified on load and rejected if not of the defined-by-interface size. (64 and 1024, respectively). Such tables should additionally verify that the table is not broken or corrupt: IE, for 3D, the axis values should be sequential and the numbers of axis values given in the first four bytes should be allowed and combine to not overflow 1024. 2D only needs to verify the axis values on load. This should likely present the user with the opportunity to fix it in future, but for now it can either reject the firmware outright or display as hex. | ||||||||||
Tags | No tags attached. | ||||||||||
Issue Type | Bug | ||||||||||
Attached Files | ![]() | ||||||||||
![]() |
|
![]() malcom2073 (manager) 2012-08-03 12:53 |
Check for size in a1f10a375cd219d7f3c646511b33afebdfc27e2b Need to add checks for axis sequentiallness, though I'm not sure that should be the concern of the tuner? |
![]() Fred (administrator) 2012-08-03 21:06 |
When two pieces of software have a contract, it is of both of their responsibilities equally. Thus I should verify that when you send me data, it's good, and you should verify that when I send you data, it's good. It's not really part of this ticket, but you should be doing it. Trust no one, especially not me, and also especially not your users. That leaves no one that isn't especially untrustworthy. You should reject my shit (likely change of protocol) as I reject yours (likely bug or incompatible version). This is just good practice. Because it's at boot up only, and you're running on a 1ghz+ processor, it's trivial to be thorough. |
![]() malcom2073 (manager) 2012-08-04 16:28 |
Done in e9ee4b26c07e62dc1cddab23c1897a388144c18e with a caveat: Since I only keep one copy of ram/flash data in memory, I am unable to check the axis values until the table actually gets loaded on a double click in EmsInfoView. If the axis are invalid, it throws an error message to the user that their firmware is broken, and does not load the table. Let me know if you want it to instead load as hex, or what. There is no way for meta-data to screw this up, so if an axis is indeed in the wrong order, it is firmware fault. |
![]() malcom2073 (manager) 2012-08-04 16:29 |
Also, I am unable to test this since I can't have broken firmware. You should test this with a custom firmware build to have invalid axis's. |
![]() Fred (administrator) 2012-08-06 16:11 |
"so if an axis is indeed in the wrong order, it is firmware fault" or who ever bypassed the checks and sent invalid data via parent location ;-) I will have a play when i get a chance. Other comments from IRC that you may lose in the haze of Ben and I discussing OLV: (16:37:38) Fred: mal|lappy: "size" : "1024" (16:37:46) Fred: from a main table (16:37:59) Fred: that's protocol knowledge right now (16:38:11) Fred: ditto 2d (16:38:18) Fred: "size" : "64" (16:39:14) Fred: "size":"2" (16:39:14) Fred: for the core vars fields, it shouldn't have a size, it should have a type, each type has a size (16:39:40) Fred: good that you don't have an overall size, though, sum of parts = whole (16:39:41) Fred: good. (16:41:09) Fred: "arraysize" : "24" (16:41:09) Fred: arraysize should be in elements not bytes (16:47:21) Fred: mal|lappy: i see what you've done re loading anyway, and pop up on click. sizes should be part of interrogation "Does what I'm getting match the file in key/all places?" If yes, load, if no, fail and produce error report. |
![]() Fred (administrator) 2012-08-06 16:19 |
Reopened and assigned back until robust/complete. |
![]() malcom2073 (manager) 2012-08-07 01:04 |
Check out f60ee8b01641cecfb0d89adc1e05f0d1bb8d2f35 You need to let me know what action you want me to take upon finding a table or axis that is invalid. |
![]() Fred (administrator) 2013-01-29 13:27 |
Attached file shows no sign of failure and just looks stalled/frozen. Turn the last one red for each section, use your best judgement. Additionally, it's dishonest. I put the bad data in the json for ID 0 and it shows OK for ID 0 and 6, when it wasn't OK for 0 and should never have gotten to 6. { "tables" : { "VE Table" : { "locationid" : "0x0000", "type" : "3D", "xtitle" : "rpm", "xcalc" : [{"type" : "div", "value" : "2.0"}], "xdp" : "0", "xhighlight" : "RPM", "ytitle" : "load", "ycalc" : [{"type" : "div", "value" : "100.0"}], "ydp" : "0", "yhighlight" : "MAP", "ztitle" : "VE", "zcalc" : [{"type" : "div","value" : "512.0"}], "zdp" : "1", "size" : "1023" }, Change "In progress" to "Pending" as in progress is wrong. We've had this discussion before, and that to me/anyone means "on the wire" not "queued waiting to be sent" I guess I'd leave the 5% bar as it is, but change it to red, and make the failed ID be red, and the Pending ones blank or failed or grey, but no longer pending. |
Copyright © 2000 - 2011 MantisBT Group |