Giter Club home page Giter Club logo

Comments (16)

bkueng avatar bkueng commented on July 29, 2024

Hi @olliw42

The idea is that an ID is unique within a given system, hence the division into component and sub ID. And the closest match that made sense to me was to use the mavlink component ID, as that is required to be distinct anyway.
Hence in your case you would use the gimbal component ID. Sub ID's you can then freely assign.
However the systemwide uniqueness isn't required so far, as the GCS queries (or rather is meant to query, as it's not implemented yet) metadata separately for different components. It only starts to matter if you were to e.g. log all events from a system in a common file.

Second, how can device-specifi/implementation-specific events, which obviously will be in their own group, ever be of any use if a UI should not display unkown groups? Again, I conclude that it all only ever could work out if one registers the event with the mavlink project.

No, the idea is that you would use the existing groups. Your events would life in your metadata json file, which is querried via COMP METADATA API. Hence the GCS does not need a priori knowledge.
Groups are meant to implement (simple) protocols, like the arming check reporting, which transmits a list of failures, so the GCS always knows the full current list.

All in all I must say it will be a fair bit of work to implement this. Certainly more than I initially thought it would be. With the library I tried to make it simple to use for others. But it doesn't include low-level embedded implementations, which I directly did in PX4.

Feel free to ping me on PX4 discord for a more direct channel. But Github is fine too.

from libevents.

olliw42 avatar olliw42 commented on July 29, 2024

@bkueng
many thx for the answer. I do have discord but I find it cumbersome, e.g not easily to search etc, so if you don't mind I would keep on here.

The idea is that an ID is unique within a given system

ah, makes sense. This answers a lot.

so, the GCS would unambigiously identify an event by recording both the message source sysid and the event ID, right?

hence the division into component and sub ID. And the closest match that made sense to me was to use the mavlink component ID, as that is required to be distinct anyway

the id recipe doesn't make fully sense to me

the message has also a source compid, so I would write down the full id as

sysid.compid.component.subID

and if as you suggest that component = component ID one arrives at

sysid.compid.compid.subID

so, I don't see why the component piece is needed, this info could equally be determined from the messages's source compid.

Furthermore, the association to components is not very convenient. I would have rather thought that there should be some "vendorID". For instance, let's assume a system made of an autopilot and a gimbal. Now the user changes the gimbal compid. The meta data file would have to change, the event ID would change, the GCS couldn't use whatever it may have cached, the metadata file could not be hosted elsewhere, e.g. on the autopilot instead of on the gimbal, as it changes. Let's now assume a system made of an autopilot and two gimbals of the same brand (yes, such systems are in use in STorM32 world ;)). The GCS would have to retrieve two metadata which essentially contain exactly the same info. And so on and on.

I note that AUTOPILOT_VERSION does already have a vendor_id/product_id pair, which, in my understanding, should be unique (though it is not clear how to ensure that, I guess the idea are using USB ids). I wonder if these could not be (re)used in some way to come to something like (the subId could easily be just 16 bits wide, giving 16 bits to vendor which should be sufficient):

sysid.compid.vendorid.subID

Q:
I do understand you correctly however in that you are saying that using compid for the copmponent is not mandatory, every one can follow her/his own rules for how to achieve system uniqueness? If so, wouldn't it maybe a viable approach to make the comp piece 16 bits wide, and the subId piece only 16 bits too (or even 24-8 bits, see below)?
Mavlink could then offer registration of the 16bit comp piece, to avoid overlap, and this value could in fact e.g. be the starting number of the CMD range in a vendor specific dialect ...

E.g., for the STorM32 it would then be 60000, and ASLUAV would be 40001, and AVSSUAS 60050 , and herelink/cubepilot 50000

This would avoid that too many numbers per vendor/dialect need to be fooled with.

If that range of 16bit comp is considered too small, one could make it 24 bits and the subId only 8 bits, but give a vendore a larger range of comp-IDs, which again might be e.g. teh message ranges associated to dialects, e.g. for STorM32 it would be 60000 - 60049, and for ASLUAV.xml 8000 - 8999, etc. pp.

the idea is that you would use the existing groups

hm ... I understand that there should be some common groups, but I guess I don't understand why a GCS shoudl not display non-common groups ... IMHO this really takes out the whole purpose of the concept. Whatever the group is I would want the GCS to display the event.

it will be a fair bit of work to implement this

I think the complexity comes because of this "garanteed delivery" aspect. I think a burst approach might be simpler, like if an event happens the source sends it 5 times within 1 sec, also with some seq number so the GCS can filter out duplicates, and only for "critical" events on coudl expand like it is send for ever unless the GCS acknowledges reception which would stop it. Maybe not fleshed out, but the current mechanism is what I think makes it complicated. (and maybe too complicated to find much practical relevance)(I mean, I can't even wrap my head around what log level I should choose ... way too many of them, and then even internal external for some reason, probably super flexible, but super complicated)(dito for the arming check, why so complicated for what is findamentally simple)

from libevents.

bkueng avatar bkueng commented on July 29, 2024

so, the GCS would unambigiously identify an event by recording both the message source sysid and the event ID, right?

Yes

so, I don't see why the component piece is needed, this info could equally be determined from the messages's source compid.

Yes you could do that, but I didn't want to tie the events to mavlink. You could have a different protocol to send events (e.g. ROS).

I'm aware of the downside you mention. I don't mind using vendor ID's instead. What's important to me is that the sub-id is 24 bits (at least for the autopilot, as it's using a hashing mechanism). 16 bits would increase chance of collisions too much.

hm ... I understand that there should be some common groups, but I guess I don't understand why a GCS shoudl not display non-common groups ... IMHO this really takes out the whole purpose of the concept. Whatever the group is I would want the GCS to display the event.

You probably think too far. Every event that should just be displayed will use the default group. They should not be separated.
A group means 'treat these events differently', hence unknown groups should just be ignored.

I think the complexity comes because of this "garanteed delivery" aspect. I think a burst approach might be simpler, like if an event happens the source sends it 5 times within 1 sec, also with some seq number so the GCS can filter out duplicates, and only for "critical" events on coudl expand like it is send for ever unless the GCS acknowledges reception which would stop it. Maybe not fleshed out, but the current mechanism is what I think makes it complicated. (and maybe too complicated to find much practical relevance)(I mean, I can't even wrap my head around what log level I should choose ... way too many of them, and then even internal external for some reason, probably super flexible, but super complicated)(dito for the arming check, why so complicated for what is findamentally simple)

The protocol itself is quite simple with some corner case handling. The effort comes from all infrastructure required.
And the other thing is if you go down into the details, these are all things we want to achieve a good UX.

from libevents.

olliw42 avatar olliw42 commented on July 29, 2024

many thx again, much appreciated

Yes you could do that, but I didn't want to tie the events to mavlink. You could have a different protocol to send events (e.g. ROS).

ah, ok, got it

You probably think too far. Every event that should just be displayed will use the default group.

you are right, I did get this wrong. Makes it even simpler, just use "default", period LOL

The protocol itself is quite simple

well, that's then subjective ;)

I'm aware of the downside you mention. I don't mind using vendor ID's instead. What's important to me is that the sub-id is 24 bits

I guessed that you wanted 24 bits for this hash to work reasonably reliably. But I maintain that linking "components" to only 8 bits and the component ID is impractical ... at least for a thing like the STorM32. It's really the thing which upholds me now from using an event to replace my prearm message.

Note that the json would be stored in compressed xz format in the STorM32 code, i.e., the compression would be done at compile time and not run time. Ergo, I can't change the "components" value at run time to adapt to the compid choosen by the user.

What can be done?

I would see only two options

  1. I use a constant value, e.g. 154 (which is MAV_COMP_ID_GIMBAL), irrespective of the gimbal's actual compid and irrespective of how many gimbals of whatever type are in the system (i.e. 154 would then kind of mean "hey, it's a gimbal")
  2. the protocol is modified to support e.g. 32bit wide "components" values, which could cannonically be set to be the VID/PID also used in AUTOPILOT_VERSION, or it could be a hash of the vendors&brands name, or whatever other to make it reasonably unique. The subID could then be also 32bit, helping your hash even better LOL.
  3. this "components" values thing is totally dropped, since as argued before, if it is the compid it could also be got from the message and is superflous, there is then also no subId, but just a events id which is 32bits wide.

Your thoughts?

BTW: The name "components" for this part of the id is somewhat unfortunate, I would say (too easy to confuse with the actual component Id in mavlink)

from libevents.

bkueng avatar bkueng commented on July 29, 2024

Can you go with 1?
As I wrote I see some value in keeping it, but I'm also not entirely against removing it.

from libevents.

olliw42 avatar olliw42 commented on July 29, 2024

yes, I could go with 1 ... BUT: the question is if you are going to go with 1 !?!

I mean, it makes only sense if you change the standard such that anyone understands that 154 = MAV_COMP_ID_GIMBAL means that it also could be 171-175 (GIMBAL2-GIMBAL6), and that by logical extension 100 means 100-105 (CAMERA1 - CAMERA6) and that 140 means 140-153 (SERVO1 - SERVO14), and so on for GPS, INS, etc. pp

which brings me to a 4th possibility I haven't seen before

  1. change the entry for "components" to allow multiple values, e.g. "components": { "154,171-175": {}} or "components": { "100-105": {}} or "components": { "66-68,154,160,171-175": {}} ... (you get the pattern :))

from libevents.

bkueng avatar bkueng commented on July 29, 2024

Yes, sure, I can do the change. I wouldn't add the complexity of multiple values.

from libevents.

olliw42 avatar olliw42 commented on July 29, 2024

ok, fantastic
so, to summarize
that standard now says (or is going to say) that

  1. For classes of components, like gimbal, camera, servo, gps, imu, and so on, the "components" value should be that of the first instance of that class, i.e. GIMBAL1, CAMNERA1, SERVO1, GPS1, IMU1, and so on.
  2. Components of equal class but different brand/make thus need to differentiate their possibly different events by unique subIDs (by 1. components of equal class need to use the same "components" value).

EDIT: I just saw again "Each namespace has a name and an 8 bit component ID. ... So events can be uniquely identified for the whole system by ID or <namespace>::<name>.".
Doesn't this necessarily imply that not only the value is fixed by the new convention, but that also the name must be fixed. I.e., for 154 the name must e.g be "gimbal", for 100 "camera", and so on?

from libevents.

olliw42 avatar olliw42 commented on July 29, 2024

maybe you allow me to grab the opportunity to continue to ask

I assume that it is not against the events microservice standard to voluntarily send out the last event at some interval, right?
(I would send out the EVENT msg with the last event along with CURRENT_EVENT_SEQUENCE, as this makes things easier for as long as the event protocol is not widely supported)

the hash function you are using to generate subID's from the event name, which is it?
(ideally there would be a python3 function I could use)

just to not make a mistake, the components value + subId are stored in the u32 as compvalue << 24 + subId & 0x00FFFFFF, right?

could you maybe have a look at "my" json file for the event I'm planing to use, if it looks as it should?
I tried to mimic what I found for PX4, but there are some entries which I don't understand
(I'm trying to mimick the msg https://github.com/mavlink/mavlink/blob/master/message_definitions/v1.0/storm32.xml#L560-L568, and flags https://github.com/mavlink/mavlink/blob/master/message_definitions/v1.0/storm32.xml#L65-L114)

{
    "version": 1,
    "components": {
        "154": {
            "namespace": "storm32",
            "event_groups": {
                "default": {
                    "events": {
                        "0": {
                            "name": "storm32_prearm_status",
                            "message": "Arguments: {1} {2}",
                            "description": "STorM32 prearm check",
                            "arguments": [
                                {
                                    "type": "uint32_t:STORM32_GIMBAL_PREARM_FLAGS",
                                    "name": "enabled_flags"
                                },
                                {
                                    "type": "uint32_t:STORM32_GIMBAL_PREARM_FLAGS",
                                    "name": "fail_flags"
                                }
                            ]
                        }
                    }
                }
            },
            "enums": {
                "STORM32_GIMBAL_PREARM_FLAGS": {
                    "type": "uint32_t",
                    "is_bitfield": true,
                    "separator": "/ ",
                    "entries": {
                        "1": {
                            "name": "IS_NORMAL",
                            "description": "STorM32 gimbal is in normal state."
                        },
                        "2": {
                            "name": "IMUS_WORKING",
                            "description": "IMUs are healthy and working normally."
                        },
                        "4": {
                            "name": "MOTORS_WORKING",
                            "description": "Motors are active and working normally."
                        },
                        "8": {
                            "name": "ENCODERS_WORKING",
                            "description": "Encoders are healthy and working normally."
                        },
                        "16": {
                            "name": "VOLTAGE_OK",
                            "description": "A battery voltage is applied and is in range."
                        },
                        "32": {
                            "name": "VIRTUALCHANNELS_RECEIVING",
                            "description": "Virtual input channels are receiving data."
                        },
                        "64": {
                            "name": "MAVLINK_RECEIVING",
                            "description": "Mavlink messages are being received."
                        },
                        "128": {
                            "name": "STORM32LINK_QFIX",
                            "description": "STorM32-Link data indicates QFix."
                        },
                        "256": {
                            "name": "STORM32LINK_WORKING",
                            "description": "STorM32-Link is working."
                        },
                        "512": {
                            "name": "CAMERA_CONNECTED",
                            "description": "Camera has been found and is connected."
                        },
                        "1024": {
                            "name": "AUX0_LOW",
                            "description": "Signal on AUX0 input pin is low."
                        },
                        "2048": {
                            "name": "AUX1_LOW",
                            "description": "Signal on AUX1 input pin is low."
                        },
                        "4096": {
                            "name": "NTLOGGER_WORKING",
                            "description": "NTLogger is working normally."
                        }
                    }
                }
            }
        }
    }
}

from libevents.

bkueng avatar bkueng commented on July 29, 2024

Doesn't this necessarily imply that not only the value is fixed by the new convention, but that also the name must be fixed. I.e., for 154 the name must e.g be "gimbal", for 100 "camera", and so on?

The name can and should be more specific. For example your project name (as in your example).

the hash function you are using to generate subID's from the event name, which is it?

I use this hash: https://github.com/PX4/PX4-Autopilot/blob/main/Tools/px4events/srcparser.py#L5. It's supposed to be good for strings. You can use any other though.

just to not make a mistake, the components value + subId are stored in the u32 as compvalue << 24 + subId & 0x00FFFFFF, right?

Yes

could you maybe have a look at "my" json file for the event I'm planing to use, if it looks as it should?
I tried to mimic what I found for PX4, but there are some entries which I don't understand
(I'm trying to mimick the msg https://github.com/mavlink/mavlink/blob/master/message_definitions/v1.0/storm32.xml#L560-L568, and flags https://github.com/mavlink/mavlink/blob/master/message_definitions/v1.0/storm32.xml#L65-L114)

The arming check reporting is a bit more involved. Basically the idea is to send a list of 'problems' to the GCS. So on the FC whenever that list changes, it emits the updated list which includes 2 summary events to mark the start and end. This is described in https://github.com/mavlink/libevents#event-groups. It doesn't go into details, so I don't expect that in itself makes it clear how to implement it.
Specifically for the modes part, I intended this to be optional for the case like yours, so the report would always apply to all modes (something to be looked at in detail).
You would define the 2 summary events, and then your enum can be individual events or be kept as an enum, that's up to you.

from libevents.

olliw42 avatar olliw42 commented on July 29, 2024

Doesn't this necessarily imply that not only the value is fixed by the new convention, but that also the name must be fixed. I.e., for 154 the name must e.g be "gimbal", for 100 "camera", and so on?

The name can and should be more specific. For example your project name (as in your example).

I would say that this can't work as it contradicts to "all gimbals should use 154" and "events can be uniquely identified for the whole system by ID or <namespace>::<name>.". Obviously, it's above my paygrade to understand the concept and how this events service is supposed to work for components.
But, the good news, I don't have to understand; it is sufficient to do what I'm being told :D
So, I use 154 and storm32-ish names everywhere - and any issues resulting from this is then not my cup of tea :)

the hash function you are using to generate subID's from the event name, which is it?

I use this hash: https://github.com/PX4/PX4-Autopilot/blob/main/Tools/px4events/srcparser.py#L5. It's supposed to be good for strings. You can use any other though.

ok.
So I use a random number generator to randomly pick a number and by accident it gave me 0. Fine. Any issue resulting from this pick is not my cup of tea :)

could you maybe have a look at "my" json file for the event I'm planing to use, if it looks as it should?

The arming check reporting is a bit more involved.

ähm, I may not have been clear. I am decidedly NOT trying to use the events micro service's arming check mechanism. I frankly find it way too complicated, don't even remotely understand it, and there isn't any support for it for components anyway anywhere and there isn't going to be support for the next 5 years. So there is no good reason to go through the endavor.
All I want to achieve is to use the EVENTS message instead of my own home-grown message, so that I can drop it (which is obvioulsy possible and working) - and use it in a way which is not breaking the events microservice. So, my question was only if the json definition file is valid. :)

from libevents.

bkueng avatar bkueng commented on July 29, 2024

I would say that this can't work as it contradicts to "all gimbals should use 154" and "events can be uniquely identified for the whole system by ID or ::.". Obviously, it's above my paygrade to understand the concept and how this events service is supposed to work for components.
But, the good news, I don't have to understand; it is sufficient to do what I'm being told :D
So, I use 154 and storm32-ish names everywhere - and any issues resulting from this is then not my cup of tea :)

:D So you don't like tea?
If you use the same metadata everywhere where you use the storm32 namespace, you're all good.

ähm, I may not have been clear. I am decidedly NOT trying to use the events micro service's arming check mechanism. I frankly find it way too complicated, don't even remotely understand it, and there isn't any support for it for components anyway anywhere and there isn't going to be support for the next 5 years. So there is no good reason to go through the endavor.
All I want to achieve is to use the EVENTS message instead of my own home-grown message, so that I can drop it (which is obvioulsy possible and working) - and use it in a way which is not breaking the events microservice. So, my question was only if the json definition file is valid. :)

I don't see why you would go through about 90% and then take a shortcut.
Are you expecting the GCS to treat your message special? The way you have it now it would just pop up every time you send it.
Your json looks largely correct, you can run it through this script to validate: https://github.com/mavlink/libevents/blob/main/scripts/validate.py

from libevents.

olliw42 avatar olliw42 commented on July 29, 2024

:D So you don't like tea?

I don't know :) I decided to stop trying to understand it, I just want to get a thing done, and as you tell me that I should use 154 and "storm32" I'll just do and move on.
And if doing what I was told to do ever creates some issues lateron because it turns out to not work as anticipated I won't consider it my fault ;)

I don't see why you would go through about 90% and then take a shortcut.

what may look like 10% to you looks like 1000% to me :)

Are you expecting the GCS to treat your message special?

no

The way you have it now it would just pop up every time you send it.

that's what I thought to have understood from the above, and is what is all good for me. Great you confirm that it's going to do as intended.

Your json looks largely correct, you can run it through this script to validate: https://github.com/mavlink/libevents/blob/main/scripts/validate.py

ah, cool, many thx !! The validation fails, raises and error jsonschema.exceptions.ValidationError: 'STORM32_GIMBAL_PREARM_FLAGS' does not match any of the regexes: '^[a-zA-Z][a-zA-Z0-9_]*_t$'. If I add _t it complains jsonschema.exceptions.ValidationError: 'IS_NORMAL' does not match '^[a-z][a-z0-9_]*$'. So, looks all a bit constraint currently, but I won't worry for now and just keep it as it is :)

To conclude & summarize:
I have substituted my own message with the above events message in my STorM32 and BetaPilot projects already, and it does the job (STorM32: http://www.olliw.eu/storm32bgc-wiki/MAVLink_Communication#MAVLink_Events). All I need to do is to make the json available through metadata mftp, but that's easy to do.
So: mission ackomplished!

MANY THX again for your help with this!!

Let's see if these events ever are going to actually pop up anywhere :)

from libevents.

bkueng avatar bkueng commented on July 29, 2024

Alright, good.
Yes the naming is restrictive to keep a common convention.

from libevents.

olliw42 avatar olliw42 commented on July 29, 2024

should we close?
if you feel yes, please do so :)

from libevents.

bkueng avatar bkueng commented on July 29, 2024

Let's update the readme before closing: #5

from libevents.

Related Issues (1)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.