Mantis Bug Tracker

View Issue Details Jump to Notes ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0000477TunixFreeEMS Pluginpublic2011-12-11 22:422012-04-22 11:15
Assigned ToFred 
PlatformAllOSAllOS VersionAll
Product Version0.9.24-SNAPSHOT 
Target VersionFutureFixed in Version0.9.24-SNAPSHOT 
Summary0000477: Odd Behaviour With Broken Firmware
DescriptionInterrogation succeeds despite timeouts and nothing returned for firmware version. Firmware version has to be at least one character to be valid, for a start, but I would expect timeouts to kill the interrogation effort. It just so happens that the interface version sneaks through the bad checksum check randomly, and everything else fails. Attaching bad firmware s19 for you to play with, checksum rX code is borked in it. But it illustrates a flaw in mtx nicely.

I would make it a bit more strict if I were you, but do whatever you think is appropriate, including closing and doing nothing. I don't want to argue about this.
Steps To ReproduceLoad attached firmware, connect.
Additional Informationfred@cheetah:~$ mtxf0

** (megatunix:889): CRITICAL **: update_ecu_controls_pf: assertion `ecu_widgets' failed

** (megatunix:889): CRITICAL **: bind_data: assertion `ecu_widgets' failed
TagsNo tags attached.
FirmwareVersionprovided in ticket attachment
Attached Files? file icon FreeEMS-0.2.0-SNAPSHOT-49-gd606603-DEV-BenchTest.s19 [^] (163,452 bytes) 2011-12-11 22:42

- Relationships

-  Notes
User avatar (0000984)
Fred (administrator)
2011-12-11 22:49

ee1c8be46497f2668b4c67121ca315bd5a153fa5 is the mtx has, btw.
dandruczyk (viewer)
2011-12-12 23:15

There's it NOTHING wrong with megatunix. the interrogation matchin is entirely specifiable by the firmware author, however he has never contributed appropriate keys to match on, thus an educated guess was made.

This "broken" firmware responds correctly to the Interface version request which is one of hte interrogation tests:

Command (Interface Version Request): returned (IFreeEMS Vanilla 0.0.0)

The interrogation profile matches on that string and that is sufficient for a match.

IFACE_VER=_REGEX_,FreeEMS Vanilla 0.0.0

Hence no problem.

If you want a more rigorous match, add ",FW_VER" to the match_on line and specify the FW_VER=... line as appropriate to match accordingly and submit a pull request. Right now those are the only two tests available, but with hte recent addition of the build OS, compiler, and other commands it would be possible to key a match based on stuff in those results, but since your docs seem to indicate these are information only, that it didn't seem necessary or even wanted to match based on those results.
dandruczyk (viewer)
2011-12-12 23:16

See previous notes for information. Take action if you want, or ignore at your leisure. It is working as configured and you've provided no need or criteria for a stricter match.
User avatar (0001007)
Fred (administrator)
2011-12-13 00:32

From the original core protocol specification, it is an error to return less than 16 bytes of firmware version. I'm planning to loosen that to 1 byte, however it's certainly an ERROR and not a success, to receive zero bytes or no response at all. It is REQUIRED for all core protocol implementing firmwares to respond to all core protocol payload types. If it doesn't, it is NOT a FreeEMS compatible firmware. IE, the above posted S19 is NOT FreeEMS compatible, despite it's name and origin. From the spec doc:

"An arbitrary string 16 to 256 bytes long uniquely describing the firmware version running on the device."
dandruczyk (viewer)
2011-12-13 02:08

Afaik that criteria did not exust when freeems interrogation was written well over a year ago....
User avatar (0001010)
Fred (administrator)
2011-12-13 10:03

I know you hate it when I do this, however I hate it when you do the above, and have no choice:

"Version 0.5 - 11th November 2008." [^]

It pays to RTFM.
dandruczyk (viewer)
2011-12-13 17:52

I used what was available from the hodgepodge of scattered docs within the freeems-vanilla code and what YOU TOLD ME TO USE on IRC. Megatunix works fine as it is, the "fals positive" is due to a relatively "lax" test based on information YOU provided. If you want a more secure match, STATE THE CRITERIA you want, otherwise leave this issue closed.
dandruczyk (viewer)
2011-12-13 17:53

DO NOT REOPEN unless providing EXACT criteria to match on.
User avatar (0001018)
Fred (administrator)
2011-12-13 18:09

User avatar (0001019)
Fred (administrator)
2011-12-13 18:27

Stop being a cock, please. Thank you in advance.
dandruczyk (viewer)
2011-12-13 18:30

If you are NOT GOING TO PUT IN CRITERIA, CLOSE THIS DAMN TICKET! There is NO POINT CONSTANTLY reopening it with no content.
User avatar (0001021)
Fred (administrator)
2011-12-13 18:34
edited on: 2011-12-13 18:36

On the contrary, this damn ticket should stay open until either MTX implements the existing protocol more completely, or until the protocol is updated and MTX implements that. I don't need you to fix anything on this right now, hence the designation of "Future" for target. What I do need is for you to not panic because there is an outstanding ticket against MTX. Let it sit here, sometimes tickets stay open for years, and clear themselves up, other times they require immediate attention. This is a "your stuff doesn't do the best possible job handling corner cases, and although it's against the spec, I don't care much, but it should be noted anyway." Do NOT close this issue again until it's fixed or some other resolution is come up with. Note, it does NOT need to be fixed anytime soon, or even ever, however this must remain here as a warning to others to expect that behaviour from MTX versions on the date of reporting and possibly earlier and later.

dandruczyk (viewer)
2011-12-13 18:39

YOU, FRED COOKE have refused to provide what you want to MATCH ON thus the issue is closed UNTIL SUCH TIME that you do. I have NO IDEA what will determine a difference in API between your firmwares, you have NOT defined or provided this within this ticket, hence the issue is CLOSED until such time.
User avatar (0001025)
Fred (administrator)
2011-12-13 18:43

FFS, Dave, seriously, leave this ticket the fuck alone. It's supposed to sit here OPEN and get updated and act as a reminder, until it's really fixed and not an issue anymore. Closing it removes it from being visible and is NOT ACCEPTABLE. Tickets stay open until REPORTER close them, understood? Stop fucking with me.
dandruczyk (viewer)
2011-12-13 18:51

Seriously? provide some fucking matching information then IF YOU WANT IT CLOSED, otherwise if not , close it yourself. The matching criteria is depending on YOU and only you
User avatar (0001028)
Fred (administrator)
2011-12-13 19:05

Check your email, and stop assigning it back to me. If you assign it back to me again, I'm pasting that fucking email here.
User avatar (0001033)
Bangbug (reporter)
2011-12-14 00:13

Dave, open a new issue log for the missing criteria if you like.
Leave this open until it is robustly resolved.
Thank you all for your time and effort.
dandruczyk (viewer)
2011-12-14 00:18

Awaiting complete details from firmware author(s) to Specify the criteria in detail to match against.
User avatar (0001039)
Bangbug (reporter)
2011-12-14 01:27

Or that, the nice thing about leaving it open then.
Fred (or other author, though they may not read this, hence NEW issue) if you could help dear Dave with his criteria matching that'd be lovely.
Once we're all back from our sabbaticals :)
dandruczyk (viewer)
2012-01-02 21:00

Nothing will be done about this until the firmware author designates appropriate complete context to match upon. He has not fleshed out and published any version number specification criteria to my knowledge, thus this is on hold indefinitely.
dandruczyk (viewer)
2012-04-01 16:51

submitter/firmware author hasn't provided info in months, marking as resolved so the submitter can either close it or provide concrete ecu-interrogation information.
User avatar (0001359)
Fred (administrator)
2012-04-01 17:05

Once again, this issue is to remain open until the existing protcol as described in up to date documents, provided to you in the past, is more completely implemented. [^] [^]
dandruczyk (viewer)
2012-04-01 20:08

please be completely SPECIFIC as to what needs to be met within those documents
dandruczyk (viewer)
2012-04-01 20:08

see previous comment
User avatar (0001498)
Fred (administrator)
2012-04-22 11:15

While performing testing with some broken hardware last night I re-verified that this is still an issue. I had the line from USB-UART tx to MCU rx *diconnected* and I got the following output:

Attempting to open port /dev/ttyACM0
/dev/ttyACM0 Opened Successfully
Searching for ECU
Search successfull

Which I just noticed has a spelling mistake in it. Unfortunately it only loaded two tabs, and because the wire was disconnected, it's not possible for it to have succeeded.

I'm not expecting a fix, I just put this here for a record.

Copyright © 2000 - 2011 MantisBT Group
Powered by Mantis Bugtracker