Mantis Bug Tracker

View Issue Details Jump to Notes ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0000416TunixFreeEMS Pluginpublic2011-11-22 11:312011-11-23 17:08
ReporterFred 
Assigned ToFred 
PrioritynormalSeverityfeatureReproducibilityN/A
StatusclosedResolutionfixed 
PlatformAllOSAllOS VersionAll
Product Version0.9.24-SNAPSHOT 
Target Version0.9.24Fixed in Version0.9.24 
Summary0000416: Add Bump And Stop Functions To Bench Test UI
DescriptionNo, I've not documented them, yet, but there are two additional controls available for bench testing:

Stop the test - replaces the reset button, though it might be useful to keep that in the UI until things stabilise.
Bump the test - requires a numeric input and allows the user to bump the test by adding X cycles to whatever is left.

These are implemented as modes in the first byte of the payload.

Stop = 0x00 only
Start = 0x01 + other data to configure a test run = body as it has always been, no change
Bump = 0x02 + one unsigned char representing 1 - 255 extra cycles. Zero gets an error, but I just realised that I'd be better to make it 1 - 256 by adding one and never error. Will change this later today after I do a few other things.

Greater than 0x02 = unimplemented error.

It really is that simple, so consider this your documentation, or just wait until I write it up in LaTex in the next week or two, your call, but don't assign back :-p
TagsNo tags attached.
FirmwareVersion
Attached Files

- Relationships

-  Notes
(0000653)
dandruczyk (viewer)
2011-11-22 15:02

can't do 1-256, as 256 is 9 bits....
User avatar (0000654)
Fred (administrator)
2011-11-22 15:06

I don't understand. It's a fundamental requirement of all of the configuration, including the tables, to display a different scale and offset to the real underlying data. What is different about one little text box here? I'm gunna do it, I hope you can support it.
(0000655)
dandruczyk (viewer)
2011-11-22 15:08

If you are using the bench test packet payload, the packet payload length you'd provided is INVALID.

i.e. a payload ID should have a consistent format no matter what...
The current benchtest packet
has mode as the first byte
events/cycle as the second byte
The cycles bits go into the next two bytes
and the ticks/event bits go into the next 2 bytes
followed by 6 8 bit values for the events.
followed by 6 16 bit values

You're telling me nothing about what needs to goto into all the other fields?
Or is this a different payload ID?

Technically your end should choke if i send the benchtest payload ID, 0x01 and a single value without all the other stuff as this overlapps the existing API in a seemingly ambiguous way.
(0000656)
dandruczyk (viewer)
2011-11-22 15:13

> I don't understand. It's a fundamental requirement of all of the configuration, including the tables, to display a different scale and offset to the real underlying data. What is different about one little text box here? I'm gunna do it, I hope you can support it.

Because THIS IS NOT a table value, this isn't a value that can be READ BACK from the device LIKE a table value can be. You'll be trying to do the same mistake jean did with the jimstim where you can upload patterns but NOT read them back , so the UI becomes out of sync with the reality of the device. This is an API call (two very different things).

The correct answer, is to make the device ignore the 0 (simple (if count == 0), then continue as if nothing happened), and use 1-255 as appropriate, otherwise you will introduce potential off by one issues within your firmware.
User avatar (0000657)
Fred (administrator)
2011-11-22 15:18

The format is dynamic for the type of bench test command that you're sending. All bench test packets share the same payloadID, and each internal-to-bench-test action ID requires different arguments. Expect the same thing from error packets in future, ie, error code first or maybe even last, and a random payload of contextual data about what went wrong.

Bench test mode is a special case anyway, just like a unit test framework, which, btw, would also work the same way, ie, test type id, and required data for that type of test, all in one payload id. There are other examples in the firmware that have been there for ages where depending on the data received it will send you an error. eg if you send 01 and less than or more than a full packet, it will respond as such, like wise 02 with less than or more than 1 extra byte. same for 00 with any data more than none. I can't remember if I added those checks or not, but I probably should to be consistent and safe in terms of fuel and workshops :-)

This has been in the code for a while, months maybe, I just realised that you didn't have it yet while working on docs.

Test packets are available:

FreeAir:interface fred$ ls -1 ../../lib/test.packets/request.bench.test.*
../../lib/test.packets/request.bench.test.assorted.bin
../../lib/test.packets/request.bench.test.parallel.bin
../../lib/test.packets/request.bench.test.sequential.bin
../../lib/test.packets/request.bench.test.to.bump.by.10.bin
../../lib/test.packets/request.bench.test.to.stop.bin
(0000658)
dandruczyk (viewer)
2011-11-22 15:23

Thats UGLY, and completely breaks mtx's wish/assumption that each payload ID would be CLEAN AND CONSISTENT and not full of if's, and's and but's. Now all requiring very special handling and very special packet assembly techniques due to the payloads being of varying size based on some PART of their content (yeecch!!!). Not a cool design.

A smart design would be a payload ID to start a test, a DIFFERENT payload ID to bump a test, and a third payload ID to stop/abort a running test.
User avatar (0000659)
Fred (administrator)
2011-11-22 15:30

That's not extensible. I could end up with 100 payload ids dirtying the overall interface for a special case, yuck.

Each payload ID defines its contents. What that content is has always been 100% free form. If a payload ID makes something conditional on something else, that's up to it. That's why you don't have to support it. You can just carry on doing it how you are, if you want. It's an optional extra.

What would you do for the errors with payloads situation? You're stuck with whatever you received in the case of those, so you've got no choice but to package the randomly structured data in the same payload id (in response to some request), where the structure is defined on a per error basis.

I disagree that it's dirty.

XML, JSON, YAML all have optional parts in exactly the same style.
(0000660)
dandruczyk (viewer)
2011-11-22 15:46

Error messages are consistent, the response packet should have the error payload ID and the code. The tuning end has a list of codes to textual representations that it presents to the user. Mtx uses sequence ID's on everything so it should be able to maintain track of issues from impropeorly sent packets, so you're either being ambiguous or just unclear.



> That's not extensible. I could end up with 100 payload ids dirtying the overall interface for a special case, yuck.

Huh? be more specific.
User avatar (0000662)
Fred (administrator)
2011-11-22 16:51

Re errors, imagine a packet like this:

[start][flags][payloadID][sequence][length][errorCode][dataOfAnyLengthAndFormatThatCausedTheError][checksum][stop]

Where the format is defined on a per error code basis.

Re 100 payload IDs for testing, I intend to provide a large number of modes, that's what the mode field was always for, since the start, there just used to be only one. If you want to send bogus data in the other parts of the packet, if it doesn't error now, I won't make it error, and it's fine, just stuff the bump value in the first byte after the mode and call it a day :-)
(0000665)
dandruczyk (viewer)
2011-11-22 20:30

Error messages are NOT supposed to return the text, only the code that maps to the text externally.. Otherwise why bother with codes at all?

I may have found a fugly way to handle shit your way, but I don't like it and it'll break in ugly ways when you change and break the API over time and there's no way around that (even with your precious layers. A break is a break, and all it does it break more shit..
User avatar (0000666)
Fred (administrator)
2011-11-22 20:47

They won't be returning text, at least, that's not what I intended, it would grow too big too fast. I'm talking about pulling data from registers and variables and state from structs and passing it back such that the reason for some errors can be accurately determined at a later date by post processing. There would be on real reason to handle this in real time, though you could provide that facility as a service to your users :-)

Re the fugly way, of course it's code and would need to change if the API changes, it's a special case, it's not a block of data to edit/save, its a functional thing. All such special purpose packets require custom code.

The layering just means that that code gets isolated in a specific place and affects nothing else if that part of the API changes.

I expect each and every payload type will have unique code backing it. I see no other way to handle that aspect of such a system. If you do, please share.
(0000669)
dandruczyk (viewer)
2011-11-23 15:41

features completed.

   9a3b907..f5f4e49 master -> master
(0000670)
dandruczyk (viewer)
2011-11-23 17:04

Resolved. awaiting fred to verify and close
User avatar (0000671)
Fred (administrator)
2011-11-23 17:08

Works as advertised! Thanks Dave! :-)


Copyright © 2000 - 2011 MantisBT Group
Powered by Mantis Bugtracker