Marcos' development comments!

Marcos Chaparro's GPL C++/QT tuning application, now supports FreeEMS too!
User avatar
Fred
Moderator
Posts: 15431
Joined: Tue Jan 15, 2008 2:31 pm
Location: Home sweet home!
Contact:

Re: Marcos' development comments!

Post by Fred »

clean tree = no pending changes, ie, nothing to commit, git status reads nothing, git diff reads nothing, etc.

I hope you didn't mind the Ingles thing... if you don't want me to do it publicly, just say so.

Good on you for the checksums thing. Somewhere I have/had some logs with the occasional bad packet in them. I wonder if i can dig them up and you can keep some statistics out of the processing stages? It'd be nice to see connection quality stats such as :

bad checksums found
total packets checked
other error type a (dodgy escaped bytes found)
other error type b (wrong length)
c, d, e, etc

spurious data between packets should probably be reported eventually, but for now, i send dual start/stops to get around some bug in the way i handle the serial device, so there will always be spurious data between packets. i see debug from your code showing that. it should almost be considered normal to get some junk between packets.

Does that all make sense?

I don't mind what you impl first, but I'd personally want to nail down the low level serial stuff and make it fast and layered so you're on top of a good foundation.

Any code for FreeEMS is good code by me! :-)

Fred.
DIYEFI.org - where Open Source means Open Source, and Free means Freedom
FreeEMS.org - the open source engine management system
FreeEMS dev diary and its comments thread and my turbo truck!
n00bs, do NOT PM or email tech questions! Use the forum!
The ever growing list of FreeEMS success stories!
User avatar
nitrousnrg
LQFP144 - On Top Of The Game
Posts: 468
Joined: Tue Jun 24, 2008 5:31 pm

Re: Marcos' development comments!

Post by nitrousnrg »

I hope you didn't mind the Ingles thing... if you don't want me to do it publicly, just say so.
No no, no problem. I still need a lot of practice here.
Good on you for the checksums thing. Somewhere I have/had some logs with the occasional bad packet in them. I wonder if i can dig them up and you can keep some statistics out of the processing stages? It'd be nice to see connection quality stats such as :

bad checksums found
total packets checked
other error type a (dodgy escaped bytes found)
other error type b (wrong length)
c, d, e, etc

spurious data between packets should probably be reported eventually, but for now, i send dual start/stops to get around some bug in the way i handle the serial device, so there will always be spurious data between packets. i see debug from your code showing that. it should almost be considered normal to get some junk between packets.

Does that all make sense?
Yep, makes sense. I'll do some statistics in the communication class, it easy. And knowing that the double start/end is a bug, that debug can be dismissed for now..
Marcos
ehb
LQFP112 - Up with the play
Posts: 128
Joined: Tue Dec 23, 2008 12:20 am

Re: Marcos' development comments!

Post by ehb »

Hey Marco again,

Hope you don't mind if I point out some things I noticed.
  • Opening a config file expects a .txt extension - when saving it should suggest saving to .txt, imho. (better .conf or something?)
  • When I open a .txt while another has already been opened, I get

    Code: Select all

    QTableWidget: cannot insert an item that is already owned by another QTableWidget
    in the console - many times.
  • I can enter non-number characters in the tuning table, hit enter and they stay there. Shouldn't be possible as re-opening the file zeroes out the field (and makes no sense anyways ;) ).
  • The 3D fuel map turns pretty quickly when dragged, and is only updated when opening a file. I guess you know the latter one, the first one is more a convenience thing :)
  • The rev counter is broken after opening a saved file (ah, entering a fuel cut value fixed it) - this value could also be used to hide unneeded columns in the VE tuning table? No need to tune for 10krpm if the engine only does 8000.
  • File->New won't work
  • Saving an unedited fiie (after starting ECUmanager) ḿakes the last two columns of the VE table be 10000 rpm
Sorry for the buglist, you're on a very good way though :)

Greets,

ehb

PS: Stupid question, but does "Iny Time" actually mean something? (the bar on the right) If yes, what? :)
User avatar
nitrousnrg
LQFP144 - On Top Of The Game
Posts: 468
Joined: Tue Jun 24, 2008 5:31 pm

Re: Marcos' development comments!

Post by nitrousnrg »

Cool, I've things to do.

Most of those bugs are known (but forgotten).

1. I thought that was fixed, and that the app used .cfg instead of txt. Apparently I didn't do it.
2. Ahhhg those crappy tables! I don't know how to solve that yet, It still works AFAIK, but it shouldn't show all those warnings.
3. Good one, it never occurred to me that a user could enter text.
4. It feels good on my laptop, I'll look in the qwtplot library, maybe there is some tunable variable. The second part is in known_issues file. It was a performance problem.
5. Probably a config file issue. Severe tough.
6. File->new should close and clean up everything. It don't do that, but if I remove it, I'll forget that there should be something that closes and cleans up everything. That is the good thing about alpha software, I can let things like that as a reminder -sorry for the inconvenience, it'll be fixed- . The beta version must be complete.
7. That sounds familiar too, I never fixed that.

ha! Iny Time should read Inj Time, as the rest of the source code. Anyway, Pulse Width (PW) is a better name for that.

I'll work on your list ehb, thanks a lot!!
Marcos
User avatar
nitrousnrg
LQFP144 - On Top Of The Game
Posts: 468
Joined: Tue Jun 24, 2008 5:31 pm

Re: Marcos' development comments!

Post by nitrousnrg »

this value could also be used to hide unneeded columns in the VE tuning table? No need to tune for 10krpm if the engine only does 8000.
The idea behind the configurable table headers is that you could remove the 8000+ rpm section by using narrower rpm pionts. Instead of using 500rpm increments, you can use 400rpm incr, for example. That way, you have a finer grained table.
The same applies to load axis. A turbo engine could pass the 200kPa, while a N/A won't exceed 110kPa.

Am I clear?
Anyway, if the users community request something, I can do it. ( lol, right now, it have as much as 5 "users" -including me, fred,you, and a couple more-).
Marcos
ehb
LQFP112 - Up with the play
Posts: 128
Joined: Tue Dec 23, 2008 12:20 am

Re: Marcos' development comments!

Post by ehb »

Yes you are clear :)
The finer grained VE table does make sense. A feature to average the RPM values between 0 and max RPM (and load axis) would be useful, but not necessary at the moment I guess. Now that I think about it, I think I've read this suggestion here a while back already. Sorry for repeating ;)

I think for a new document (or configs that miss certain mandatory values), some kind of wizard would be useful. Just for the necessary basics so the program works properly.

I used FreeMind to make a mindmap with some suggestions* how this could be handled. I don't know if the injector stuff is supposed to work like this in FreeEMS, I just thought it was a nice idea. Point 4 is not needed to make a VE table, just to actually run an engine, so it can be omitted from the "New config" wizard.

Great work with the bugs :) Do you have any kind of automatism to build the debian packages? At some later point, a ppa with always the latest ECUmanager would be awesome ;)


Proud to be one of the first five users,

ehb

*Fred, I can't upload an image. If you get the time, you maybe could fix this please. :)
User avatar
nitrousnrg
LQFP144 - On Top Of The Game
Posts: 468
Joined: Tue Jun 24, 2008 5:31 pm

Re: Marcos' development comments!

Post by nitrousnrg »

A feature to average the RPM values between 0 and max RPM (and load axis) would be useful,
A function to smooth the table should be delivered with QuickTune. Not now, when we have "real" users (users with an engine using FreeEMS)
I think for a new document (or configs that miss certain mandatory values), some kind of wizard would be useful. Just for the necessary basics so the program works properly.
Yep, thats a good idea. Qt has wizard classes :-).
It'll be implemented anyway, but not now. Thanks for your time designing the wizard! I didn't knew FreeMing, its pretty useful for documentation purposes ;-)

Besides of fixing minor GUI bugs, I'm working in the communications thread. I want this app to be able to burn and get all tables available in FreeEMS, as well as resolving custom datalogs. Its critical, without that ECUmanager is useless.
Do you have any kind of automatism to build the debian packages?
Not at all.
These are my commands to build the package:
marcos@nitrousPresario:~/ECUmanager$ fakeroot dpkg --build debian/
marcos@nitrousPresario:~/ECUmanager$ dpkg-name debian.deb
marcos@nitrousPresario:~/ECUmanager$ lintian ecumanager_0.6_i386.deb <- just to check it
The tedious part is making the debian/ directory, and especially the man page and changelog.

I'd like to see ecumanager as a part of the debian official repository. I know a guy that made it with his app. After that, maybe it'd be included in ubuntu automagically.

Again, thanks for your concern. See you soon ehb
Marcos
User avatar
Fred
Moderator
Posts: 15431
Joined: Tue Jan 15, 2008 2:31 pm
Location: Home sweet home!
Contact:

Re: Marcos' development comments!

Post by Fred »

With respect to deb/unbuntu, if it ever becomes rock solid stable, and doesn't require a rebuild to work with a new firmware version, then sure. Otherwise having your own deb repo (could be on my server?) that is kept up to date by you and provides for each major deb distro would be the winner :-)

With regards inj time and PW, consider dead time, and multiple shots per cycle. You want to display something like "total fuel delivery time" which I provide as a value in the log, I think. And also actual PW which includes dead time, and combined with number of shots per cycle, and time per cycle, gives duty cycle %.

Fred.
DIYEFI.org - where Open Source means Open Source, and Free means Freedom
FreeEMS.org - the open source engine management system
FreeEMS dev diary and its comments thread and my turbo truck!
n00bs, do NOT PM or email tech questions! Use the forum!
The ever growing list of FreeEMS success stories!
User avatar
Fred
Moderator
Posts: 15431
Joined: Tue Jan 15, 2008 2:31 pm
Location: Home sweet home!
Contact:

Re: Marcos' development comments!

Post by Fred »

nitrousnrg wrote:For the record:
Image

Extra start/stop bytes between packets are errors, and will be displayed. There's no need to have byte transmissions not contemplated in the original protocol. Sorry Fred :-)
There is no such thing as an extra stop byte. An extra start byte is no different to any other partial packet that starts with a start, it's just a minimal case. It worries me that you can state how many extra stop bytes there are when there is no point searching for them unless you're in a packet, at which point, a stop means stop and anything after it is ignored / some-other-error) What you probably want to stat are :
  1. Good packets
  2. Bad checksums
  3. Length mismatch
  4. Partial packets
  5. Bad data between packets
  6. Escaped byte invalid
And maybe some other things. As with code, names matter! :-)

Fred.
DIYEFI.org - where Open Source means Open Source, and Free means Freedom
FreeEMS.org - the open source engine management system
FreeEMS dev diary and its comments thread and my turbo truck!
n00bs, do NOT PM or email tech questions! Use the forum!
The ever growing list of FreeEMS success stories!
User avatar
nitrousnrg
LQFP144 - On Top Of The Game
Posts: 468
Joined: Tue Jun 24, 2008 5:31 pm

Re: Marcos' development comments!

Post by nitrousnrg »

With respect to deb/unbuntu, if it ever becomes rock solid stable, and doesn't require a rebuild to work with a new firmware version, then sure. Otherwise having your own deb repo (could be on my server?) that is kept up to date by you and provides for each major deb distro would be the winner :-)
Lets see where it is going. The debian compliant packet is ready, it can even be built from source in a debianized way (using debuild). A local repo is likely to be the choice before the app is stable and after we have users to use it ;-)
With regards inj time and PW, consider dead time, and multiple shots per cycle. You want to display something like "total fuel delivery time" which I provide as a value in the log, I think. And also actual PW which includes dead time, and combined with number of shots per cycle, and time per cycle, gives duty cycle %.
Yep, my Inj Time (now PW) is the total fuel delivery time, and my duty % is what you said. If you think it could have a better name, tell me and I'll change it. You shouldn't leave the naming in hands of a inexperienced latin american :P
What you probably want to stat are :

Good packets
Bad checksums
Length mismatch
Partial packets
Bad data between packets
Escaped byte invalid
Good good! Those are much better than mine.

I find extra stop bytes because of the way I search the stream, starting backwards from the first stop byte occurrence (instead of searching forwards from the first start occurrence, as you do). Don't mind me, any data between packets is bad, no matter if it is a random byte, or a start/stop one.

see ya
Marcos
Post Reply