Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

VT server support #135

Closed
tomstix opened this issue Jan 17, 2023 · 13 comments · May be fixed by #400
Closed

VT server support #135

tomstix opened this issue Jan 17, 2023 · 13 comments · May be fixed by #400
Labels
enhancement New feature or request

Comments

@tomstix
Copy link
Member

tomstix commented Jan 17, 2023

I was thinking about starting to work on a virtual terminal server. This could be helpful for development to test applications without the need of an actual terminal. Also it could be used, together with some kind of CAN to WiFi adapter, to use a phone/tablet as VT.

To do so, we should talk about how and how much of the vt server part should be included in this library and how to structure it.
What I have in mind is to add some kind of object pool parser to the library. This creates instances of the different object types which the actual application can then use to draw the virtual terminal content. This could also be useful for editing object pools. Also, the library should provide functions and callbacks for interaction with the vt server.

I was actually starting to write some code when I realized this intersects with the existing VT client code and also with what @ad3154 is doing right now with adding auto-scaling to VT objects.

So, what are your thoughts on how to implement this? Having a VT base class and then subclass it for server and client? Or have all the object pool related functions elsewhere?

@tomstix tomstix added the enhancement New feature or request label Jan 17, 2023
@ad3154
Copy link
Member

ad3154 commented Jan 17, 2023

VT server itself is a tricky one, mostly I think because in order to actually draw to the screen we'd need to start adding external dependencies or platform specific (or at least graphics API specific) code, and I would hesitate to start adding a bunch of even optional dependencies on some graphics library to this repo. So my gut reaction is that we might want a full VT server to be a separate repo - a full application that integrates this library for the CAN pieces (and probably requires some additions to this library) but ultimately keeps things like graphics APIs out of here. Keeping the stack small-ish and dependency free is likely to help us drive widespread adoption.

But, an object pool parser and maybe a VT server class of some kind could be nice in here to give those applications a good starting point? Not sure. The code size for something like that could be quite large, so we may still want to make it an optional component, IDK but that's a minor detail.

For what it's worth, I have started a VT server as a side project that uses this library, a modified version of Dear Imgui, SDL2, spdlog, and OpenGL3 to make a full application. It is not even at a prototype stage yet, but it at least parses out the IOP objects into C++ objects up to VT version 5 (well, excluding macros. I hate macros. I'll add them eventually...), does a fair bit of error checking, and renders a basic window. There's a lot left for me to do on it though, as it will certainly not render things properly until I make tweaks to the Imgui layer (but it's a start). I hesitate a bit to share it in its current, not really functional state but I would consider pushing it up to GitHub if its something we wanted to collaborate on or if you want to take a look at the object parsing at least.

That being said, it's just a fun side project, so if you are interested in making your own, or adding an object parser to this project of your own creation, that is absolutely OK also - I do not have a lot of experience with GUIs (I'm an embedded firmware guy by trade) so you may have a better approach than I do. Plus I'm pretty busy recently haha, so if you have more time than I do that may also be a reason to take a crack at it.

@tomstix
Copy link
Member Author

tomstix commented Jan 17, 2023

I totally agree that this library should be kept free of any graphics library etc. but only provide the interface/abstraction layer to ISO11783.
What I had in mind was a Qt application for everything GUI that uses this library for interfacing and maybe also for object pool parsing. Later on, this could be extended with AUX-N assignments, Task Controller, etc.

I was mainly wondering, if the whole object pool parsing etc. could be helpful for VT client applications (scaling, adjusting to VT versions etc.)? And how code written for VT client (enums etc.) can best be reused.
Making the VT server optional is definitely a good idea.

This is also just a hobby project for me - I'm neither a GUI guy nor a really experienced programmer (mechanical engineer by trade) but I'd like to give this a try. I would be interested in what you already did on that topic or also to collaborate on your project. As I said I'm not a GUI guy , I just have a bit of experience with Qt, but I think that would definitely be helpful.

@ad3154
Copy link
Member

ad3154 commented Jan 17, 2023

Qt is a great framework for something like this - makes sense. As far as object pool parsing goes, it's a balancing act I think. On the one hand, yes a parsing library would help with things like autoscaling, or checking the pool for errors, etc. But, I do worry about RAM constrained platforms (think ESP32, STM32, etc) - we're not going to be able to hold a big tree of C++ objects in memory to manipulate them most likely, which is why the stack provides a way to get the object pools(s) in chunks instead of loading the whole thing into RAM. So, I think if we do add parsing, if we want it to be useful for autoscaling, we would want to be able to deserialize one single object, manipulate it, then reserialize it, one at a time. And importantly, for PictureGraphics, we will want to do that somehow without holding an entire bitmap in memory, which will be fairly challenging I think, which is partially why I'm trying to come up with some still-serialized read-ahead buffer thing right now for the auto scaling - mostly for RAM constrained platforms.

For a server though, those concerns are unimportant - tons of RAM will be available, so it can parse them into C++ objects easy-peasy.

So I guess what I am saying is, yeah I agree we could add a VT server interface of some kind, and I agree we should share things like enums and multiplexer bytes etc. That would be totally nice to have and we should probably do it. But, I don't know if we'll want to cross the streams with the client too much, as it may be costly from a resource perspective for a little microcontroller running as part of a working set or something.

@CRalliNA
Copy link

Hi, just thought I'd chip in here. I would also be very keen for expansion towards the VT server side of things. I've been poking around evaluating the best way of adding the functionality for my own use case and writing a small amount of code to test certain parts but it sounds like you've both got further than me.

I also have more experience in the embedded software and have limited knowledge of GUIs, but would be very willing to help in any way I can.I can also get access to ISObus implements if any real world testing is required.

@tomstix
Copy link
Member Author

tomstix commented Jan 19, 2023

I totally get your point, @ad3154 and I agree that the target hardware of VT client and server makes for different approaches in object manipulation.
So what I am thinking of / doing right now is to implement separate classes for each VT object type with their own (de)serialization functions and implement the whole Object Pool as a std::map with the object ID and a pointer to the object. This is, except enums, multiplexer bytes etc., separate from the current VT client implementation.
This way, one can decide in an end-application wether to use parsing to C++-objects for manipulation or not.

@CRalliNA I haven't actually written that much code myself either. I am currenty working on the object parsing in this branch. If you have any help or suggestions, I'd appreciate that.

@CRalliNA
Copy link

CRalliNA commented Feb 1, 2023

Hi @tomstix, sorry for the delay, I've been rather busy with other things. That looks like a good start, where have you got to now and how can I contribute most effectively?

@tomstix
Copy link
Member Author

tomstix commented Feb 1, 2023

Hi @CRalliNA, I just pushed my latest changes in my branch, but I did not have too much time either to further work on it. Looks like I won't have time to do much in the next few weeks as well (exams). So I guess you can decide yourself what and how you would like to contribute. You can keep implementing more Object Types to the parser or also what has to be done is to get all the communications working to actually receive the object pool and commands etc. from ECU to VT.

Also, if you or anyone else has any suggestions on general/structural/datatype/etc. improvements, please let me know. I don't have too much experience implementing such things yet.

@CRalliNA
Copy link

CRalliNA commented Feb 1, 2023

Ok great, I'll have a look into it

@dlips
Copy link

dlips commented May 4, 2023

Hi everyone, how is the progress on this topic? I would also like to contribute. At the moment I still need to get more familiar with the isobus++ library network parts but I could continue adding Object Types to the parser. Is your branch still the latest version @tomstix ?

@ad3154
Copy link
Member

ad3154 commented May 4, 2023

Feel free to check out this branch as well https://github.com/ad3154/Isobus-plus-plus/tree/vt-server-managed-working-set which has most of the basic objects defined and a working parser (but only tested with a small, basic object pool). It still needs work, as the CAN interface part of it isn't in there yet, and I'm sure we'll probably want to tweak the interfaces of some of the classes as we use it. Ideally it would be nice to combine these two efforts into one group effort

@dlips
Copy link

dlips commented May 4, 2023

Oh, that looks good. Unfortunately, I need to admit to myself that my C++ has become very rusty :( Nevertheless, while I need some time to study your code, I will read the isobus documents to understand the communication messages/flow and think about how to hook it up to the parser. I hope it is ok if I asked some questions here:

  • I like you approach with the threaded parser. I guess it is to not block the main thread to prevent violating the time constraints like the heart beat with the VT status message?
  • Is there a reason why the parser is written as a single function?
  • Is the goal to extend the interfaces of the VTObjects so the CAN communication is completely abstracted away? e.g. possibility to just call a method on a Button object and it automatically sends the appropriate command messages to the working set.
  • Is there some other place to asked questions in general about isobus++? It feels kind of wrong to clutter this issue with my beginner questions...

Anyway, thanks already for all the effort. What I have seen so far looks quite nice.

@ad3154
Copy link
Member

ad3154 commented May 5, 2023

Rusty is no problem, haha. We certainly welcome contributions, and can definitely provide feedback in reviews or less formally if needed. I suggest checking the contribution guide and just generally trying to match the style of the repo where possible.

I guess it is to not block the main thread to prevent violating the time constraints like the heart beat with the VT status message?

That was the goal, yeah, since there can be 65535 objects in the pool, all with relative metadata, it might take some time to fully validate all objects are both well structured and refer to extant, allowable objects based on the version of the working set etc, which is a lot to manage, so using a thread seemed safest to avoid blocking whatever code manages the CAN messaging. There's some other weird things that can happen, like an object pointer causing significant parsing recursion that might be nicer to contain in a thread.

Is there a reason why the parser is written as a single function?

Not particularly. Just keeps worker_thread_function short. Open to other approaches on that.

Is the goal to extend the interfaces of the VTObjects so the CAN communication is completely abstracted away? e.g. possibility to just call a method on a Button object and it automatically sends the appropriate command messages to the working set.

Good question, I guess I was hoping that the VT objects at some level could stay fairly generic so that we could add serialization functions to them as well, enabling the same classes to be used for creating object pools programmatically (to enable creation of GUI tools to create object pools). I don't know if it's quite there yet in terms of the API to make that happen, but that's at least what my idea was. That would mean that the CAN interaction would have to happen in some sort of higher-level class that doesn't exist yet I suppose? Or some polymorphism for the server objects? Lots of options there I think.

Is there some other place to asked questions in general about isobus++?

Feel free to open a discussion on our discussions page, that's a pretty good way to avoid a bunch of general things in an issue. There's a fair bit of code in the library all with different amounts of tests and some more consistent than others, so it's totally OK to have questions. The questions I get from others helps me know what needs to be added to the tutorials (which generally also could use some expansion).

@tomstix
Copy link
Member Author

tomstix commented May 5, 2023

Hey @dlips!
Yes, my published branch is still the latest version (unfortunately). I did play around a little bit with Qt/QML to find a way how to do the GUI side of things but there is not really anything I can show.

@Open-Agriculture Open-Agriculture locked and limited conversation to collaborators Jul 12, 2023
@tomstix tomstix converted this issue into discussion #287 Jul 12, 2023
@ad3154 ad3154 linked a pull request Dec 29, 2023 that will close this issue

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants