Traffic graph does not show the bandwidth of IPS enabled interfaces

Important notices
Before you add a new report, we ask you kindly to acknowledge the following:
[x] I have read the contributing guide lines at https://github.com/opnsense/core/blob/master/CONTRIBUTING.md
[x] I have searched the existing issues and I'm convinced that mine is new.
Describe the bug
The "Traffic Graph" does not show the current bandwidth of physical interfaces, on which IPS is enabled and no VLANs are configured. The table below the graph shows the correct bandwitdh.
As soon as I disable IPS (not IDS) or remove the affected interface from the IDS/IPS settings, the bandwidth is shown.
Physical interfaces, which have additional VLANs configured, are both shown correctly.
This happens since I've switched to a new hardware appliance with Intel network interfaces.
On my old hardware appliance with Realtek interfaces and the same IDS/IPS and VLAN configuration, the graph shows the bandwidth for all interfaces correctly.
I've found another user in the forum which is having the same issue (3): https://forum.opnsense.org/index.php?topic=18496.msg84177#msg84177
To Reproduce
Steps to reproduce the behavior:

Go to "Services: Intrusion Detection: Administration"
Enable IDS and IPS.
Add a physical interface which has no VLANs configured
Switch to the "Traffic graph"
As soon as all IDS/IPS rules are loaded, the bandwidth in the graph for the interface drops down to zero
The table at the bottom shows the correct current bandwidth

Expected behavior
No idea :)
Screenshots

Relevant log files
Please let me know if you need a log file to identify the issue.
Environment
OPNsense 20.7.1-amd64
Intel(R) Core(TM) i5-6200U CPU @ 2.30GHz (4 cores)/NRG IPU662
Networ Interfaces: Intel i211AT

Best try the test kernel when it comes out (https://forum.opnsense.org/index.php?topic=17363.msg84952#msg84952), although I'm not sure if this issue already has an upstream fix. Intel 211 doesn't seem to obey the bpf zerocopy toggle (https://docs.opnsense.org/troubleshooting/network.html). We're not using i211 ourselves, i210 works like a charm.

Same with OPNsense 20.7.3-amd64.

tried the new netmap test kernel ? (https://forum.opnsense.org/index.php?topic=19175.msg88695#msg88695)

@AdSchellevis Ooops, sorry. I didn't get an info about your answer/request.
I've installed the 20.7.4 update today, still no WAN traffic shown in the traffic graph.

@JasMan78 hmm, this is odd, I really thought I tested the graph with an i210 and zerocopy_enable (which already worked), but it seems that the new netmap kernel broke bpf in this case as well now. I probably didn't wait long enough for suricata to startup, maybe @muratbalaban43  has an idea, I think they where also looking into this subject.

@AdSchellevis , you're right. We are on this. Looks like netmap comes into the scene earlier than bpf.

@muratbalaban43 it's odd, using an earlier kernel I can't seem to get a functional bpf at all anymore (on my intel i210), so either my initial tests where flawed or I'm overlooking something completely different.

@AdSchellevis thanks for the update. Hopefully, I'll have some news next week.

I encountered this behavior when I enable promiscuous mode on Suricat running only on the physical WAN interface, igb0. disabling promiscuous immediately rectified the issue.

Same with e1000 AND vmxnet3 on VMWare 6.5.0 with 20.7.4

Seeing this exact behavior with em drivers too. Intel 82574L

Please see https://github.com/opnsense/src/pull/95 for a fix to this issue. Keep in mind that if a driver is NOT integrated with iflib, the driver is responsible for supplying its statistics to the ifnet structure. Based on this, they may or may not cause the traffic graph to flatline.
All drivers mentioned in this issue are integrated with iflib, as such they should now report traffic in IPS mode.
Tested and confirmed on e1000.

Please see https://github.com/opnsense/src/pull/95 for a fix to this issue. Keep in mind that if a driver is NOT integrated with iflib, the driver is responsible for supplying its statistics to the ifnet structure. Based on this, they may or may not cause the traffic graph to flatline.
All drivers mentioned in this issue are integrated with iflib, as such they should now report traffic in IPS mode.
Tested and confirmed on e1000.


Refactor the code

Pull Request Test Coverage Report for Build<PHONE_NUMBER>

163 of 178 (91.57%)  changed or added relevant lines in 8 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage decreased (-0.4%) to 90.377%

Changes Missing Coverage
Covered Lines
Changed/Added Lines
%

src/sequence.ts
25
30
83.33%

src/key.ts
24
34
70.59%

Totals

Change from base Build<PHONE_NUMBER>:
-0.4%

Covered Lines:
184

Relevant Lines:
200

💛  - Coveralls


Rework player controller

Still fixing some bugs:
- guns animations
- sounds
- dash
Still need to finish cleaning playercontroller.h

Task linked: CU-mb5m9k Rework player controller / AI scripts


Enter your name option

Add the option where user can enter there name and also show there name into the sidebar into the whole game.

Hello, can I work on this?

yeah

@vijaychandra1910 any update?

nope please unassign me and assign to someone.


Run net-istio tests on KinD with non-default cluster suffix

This patch adds net-istio tests for KinD on github action.
The test also uses random cluster suffix.
Fixes https://github.com/knative-sandbox/net-istio/issues/343 https://github.com/knative-sandbox/net-istio/issues/344
/cc @ZhiminXiang @JRBANCEL @tcnghia

Very nice! Thanks @nak3 !


Jalaali (Persian) Date input support

resolving  issue #1204

Is this something that could be done with inheritance?  By making GetShortFormat and maybe the Date property virtual (perhaps combined with making shortFormat and longFormat public (or readonly/protected)?
Then users could create their own custom date formats and calendars types? Could even have a Jalaali version of DateField (either in core or Scenarios) to show how to implement.

Is this something that could be done with inheritance? By making GetShortFormat and maybe the Date property virtual (perhaps combined with making shortFormat and longFormat public (or readonly/protected)?
Then users could create their own custom date formats and calendars types? Could even have a Jalaali version of DateField (either in core or Scenarios) to show how to implement.

Sounds good! It needs to change some methods identifiers in DateField class  and set them as virtual methods  to be able to override them in the inherited class.

Is this something that could be done with inheritance? By making GetShortFormat and maybe the Date property virtual (perhaps combined with making shortFormat and longFormat public (or readonly/protected)?
Then users could create their own custom date formats and calendars types? Could even have a Jalaali version of DateField (either in core or Scenarios) to show how to implement.

Sounds good! It needs to change some methods identifiers in DateField class and set them as virtual methods to be able to override them in the inherited class.

this seems like a good approach.

Hi @inamvar - We'd love to integrate this PR. Will you have a chance soon to address the changes requested?

@inamvar - are you still interested in helping with this?


How to debug key mapping or auto commands

Hi, I'm trying to debug some plugins I've written, which inevitably use some key mapping and automatic commands, but I can't debug it.For example, here is the file I want to debug, I hit the breakpoint to the third line, when I run lua require'dap'.continue() and then run lua require'dap'.step_into(), I get the message: No stopped threads. Cannot move
vim.keymap.set('n', '<BS>',
    function()
        print("start")
        for i=1,10 do
            print(i)
        end
        print("end")
    end
)

This is my dap config
local ok, dap = pcall(require, 'dap')
if not ok then
    vim.notify('load module dap faild')
    return
end

local nvim_server
local nvim_chanID

local function dap_server(opts)

    assert(dap.adapters.nlua,
        "nvim-dap adapter configuration for nlua not found. "..
        "Please refer to the README.md or :help osv.txt")

    if nvim_chanID then
        local pid = vim.fn.jobpid(nvim_chanID)
        vim.fn.rpcnotify(nvim_chanID, "nvim_exec_lua", [[return require"osv".stop()]])
        vim.fn.jobstop(nvim_chanID)
        if type(vim.loop.os_getpriority(pid)) == 'number' then
            vim.loop.kill(pid, 9)
        end
        nvim_chanID = nil
    end

    nvim_chanID = vim.fn.jobstart({vim.v.progpath, '--embed', '--headless'}, {rpc = true})
    assert(nvim_chanID, "Could not create neovim instance with jobstart!")

    local mode = vim.fn.rpcrequest(nvim_chanID, "nvim_get_mode")
    assert(not mode.blocking, "Neovim is waiting for input at startup. Aborting.")

    vim.fn.rpcrequest(nvim_chanID, "nvim_command", "packadd one-small-step-for-vimkind")

    nvim_server = vim.fn.rpcrequest(nvim_chanID,
        "nvim_exec_lua",
        [[return require"osv".launch(...)]],
        { opts })

    vim.wait(100)

    return nvim_server
end

dap.adapters.nlua = function(callback, config)
    if not config.port then
        local server = dap_server()
        config.host = server.host
        config.port = server.port
    end
    callback({ type = 'server', host = config.host, port = config.port })
    if type(config.post) == 'function' then
        config.post()
    end
end

dap.configurations.lua = {
    {
        type = "nlua",
        request = "attach",
        name = "debug current file",
        post = function()
            dap.listeners.after['setBreakpoints']['osv'] = function(session, body)
                assert(nvim_chanID, "Fatal: neovim RPC channel is nil!")
                vim.fn.rpcnotify(nvim_chanID, "nvim_command", "luafile " .. vim.fn.expand("%:p"))
                dap.listeners.after['setBreakpoints']['osv'] = nil
            end
        end
    },
    {
        type = 'nlua',
        request = 'attach',
        name = "attach to nvim",
        host = function()
            local value = vim.fn.input('Host [<IP_ADDRESS>]: ')
            if value ~= "" then
                return value
            end
            return '<IP_ADDRESS>'
        end,
        port = function()
            local val = tonumber(vim.fn.input('Port: '))
            assert(val, "Please provide a port number")
            return val
        end,
    }
}

Just lua require'dap'.step_into() or lua require'dap'.step_over() instead of lua require'dap'.continue()  and lua require'dap'.step_into()?

I tried just executing lua require'dap'.step_into() and nothing happened, don't we need to use lua require'dap'.continue() to start the dap server?

As a side note, I suggest using the new configuration that is available on the README, it avoids putting the port number everytime.
It looks like the function dap_server() is starting an embbeded nvim to launch the server which is itself running a neovim instance. It's probably better to use the standard method explained in the README, help doc or here. Let me know if it still doesn't work because it should.

As a side note, I suggest using the new configuration that is available on the README, it avoids putting the port number everytime.
It looks like the function dap_server() is starting an embbeded nvim to launch the server which is itself running a neovim instance. It's probably better to use the standard method explained in the README, help doc or here. Let me know if it still doesn't work because it should.

Thanks for the tip, it turns out that the quiet mapping is set to the embedded nvim, I can now debug normally after using the tutorial in the readme

Good, have fun debugging with osv 😄


Use cursor grab / mouse lock api

https://docs.rs/winit/latest/winit/window/struct.Window.html#method.set_cursor_grab
https://developer.mozilla.org/en-US/docs/Web/API/Pointer_Lock_API
For example it could be awesome to lock pointer when setting DragValue for example in cases when UX breaks when widget is near screen edge.

one caveat is that at least Chromium constantly shows a warning that the cursor is hidden which becomes annoying really quick


Feature request: The conversion of properties and yaml should be a relatively common function

Good idea.
It's easy for regular properties files, but it will be a bit tricky for Spring properties, per example for Spring profiles support. Example:
logging.file.name=myapplication.log
                xxx.property=health, info, refresh, shutdown, selftest, listProcessors, foo, bar
                #---
                spring.config.activate.on-profile=dev
                spring.datasource.password=password
                spring.datasource.url=jdbc:h2:dev
                spring.datasource.username=SA
                xxx.property=devValue
                #---
                spring.config.activate.on-profile=prod
                spring.datasource.password=password
                spring.datasource.url=jdbc:h2:prod
                spring.datasource.username=prodUser
                xxx.property=prodValue

becomes
                logging:
                    file:
                        name: myapplication.log
                xxx:
                    property: health, info, refresh, shutdown, selftest, listProcessors, foo, bar
                ---
                spring:
                    config:
                        activate:
                            on-profile: dev
                    datasource:
                        password: password
                        url: jdbc:h2:dev
                        username: SA
                xxx:
                    property: devValue
                ---
                spring:
                    config:
                        activate:
                            on-profile: prod
                    datasource:
                        password: password
                        url: jdbc:h2:prod
                        username: prodUser
                xxx:
                    property: prodValue

Every profile is separated by a "#---" comment line, which allows duplicated keys.
I developed something for regular and Spring properties, but I'm wondering if other frameworks need additional support... At least, I will publish this first implementation and I will see if somebody opens a new issue 😁


[WIP] Convert Canvas Widget to Use a Drawing Stack

In #319 we discussed that it was desired to remove the callbacks created by using an on_draw handler, and instead push drawing commands to a stack to be drawn with.
PR Checklist:

[ ] All new features have been tested
[ ] All new features have been documented
[X] I have read the CONTRIBUTING.md file
[X] I will abide by the code of conduct

Questions:
The stack (called "draw_commands") is stored in the implementation right now, but the determination of whether to add or delete from the stack is made on the interface. I didn't like my initial attempt with everything on the implementation, since there was a lot of repetition in methods. But do you think it is better to keep the interface more clean? I would really appreciate any feedback in this area.
Current status: the canvas widget is working properly and draws to the screen, but all tests are broken
[ ] Fix tests
[ ] Further testing with other contexts besides the default one

@freakboy3742 Thanks again for the great feedback. Here I am stuck in my little bubble of what canvas APIs provide now, and you are thinking outside the box - mind blown!
You are spot on with the remove parameter, that won't work 😄
I noticed in your Circle example that you were moving a level of abstraction up, so instead of providing low level drawing operations like an arc, we are providing an interface to draw a circle. To follow those breadcrumbs, maybe we would want to provide more standard vector tool like drawing capabilities like Rectangle, Rounded Rectangle, Ellipse, and a Pen tool. We might be able to provide a few other common drawing elements like arrows for UML or other diagramming.
I think I understand the basic concept of making a character in a game a context and translating it around the screen, I'm not sure what API someone making a game like that would like though. So that sounds like an interesting thing for me to look in to a little more.

The suggestion about "Circle" wasn't really meant as a specific API suggestion - it was meant more as an example of how you might have drawing instructions that abstract underlying drawing operations. Each of the primitives will almost certainly have a drawing instruction associated with it; but there might be other useful drawing instructions that encapsulate one or more underlying primitives, some with fixed values where the underlying primitive allows configuration.
That said - "Circle" is one of those time where we can try to educate everyone and say "a circle is just an arc with a start and end angle of 0"... or we can just provide a "draw circle" API :-) As a first pass, we don't have to capture all those use cases, though - just translating the primitives would be plenty.
Providing more complex objects like "Circle", "Rounded Rectangle" or "Arrow" as a proof of concept for more complex operations might be a nice to have, but could certainly wait until a later iteration.
Application-specific complex shapes like "UML Class" definitely sounds like something that should be left to end-users.

@danyeaw Are you familiar with Tkinter's canvas? Docs link. It came to mind because it also uses a "drawing stack", so it might be interesting to glean from their API.
Tkinter does something quite curious for 2d coords: they use complex numbers! So (10px, 45px) would be represented by 10 + 45j. Complex number math works kinda like vector math, which means that you can just add to coords together: (10 + 45j) + (0 + 5j) == 10 + 50j. I'm not sure if that's the right move for Toga, but it is intriguing nonetheless.
The API has the idea of "canvas item" objects that represent lines, arcs, images, etc.. When you call canvas.create_line(...), a line instance is added to the draw stack, and the function returns its ID so you can modify/delete it later.
Personally, I favor our approach of dealing directly with classes such as Circle and Image instead of Tkinter's approach of passing around IDs to items and using methods to indirectly modify them, and Tkinter's system seems less extensible since you can't define your own subclasses of drawables.
Anyway, I though I'd bring it up as a possible source of inspiration. Great work so far @danyeaw !

@DariusMontez Thanks! I'll take a look at those Tkinter docs.

@DariusMontez The "return an ID" approach used by Tkinter is a function of Tk itself - in Tk, everything is a string, so using an ID is pretty much the only way to refer to a complex object. Since we're using Python, there's no need to emulate that particular quirk :-)
I can see the analog between complex numbers and points, and using a builtin "point-ish" data type is definitely appealing; but I'm not sure I'm wild about leaning on that feature too hard. Consider the math of increasing the height of a widget by Y pixels - you'd need to use height + (y * j) if y is entered as an integer, but height + y if it was entered as a complex value. You could use height + abs(y) if it's ambiguous - but the abs() calculation could also accept a point offset, which would then return the wrong value. Or, you could use height + y.imag - but that then relies on you understanding the conceit that is being used. That's a source of potential error for every height calculation that is avoided if you just use a tuple.

@freakboy3742 Since for me a picture is worth a 1000 words, is this what you had in mind? I want to make sure the concept is concrete for me before I start hacking at some code. This would move the majority of the drawing methods to drawing object classes. The stack would then be a drawing_objects list on each context in the implementation level. What did you mean above by a Python Context Object? Do you mean a Python context manager (with statement) or something else?

@danyeaw I think we're mostly on the same page.
The calls to implementation layer drawing methods would be on the drawing objects. The interface layer canvas would still have draw_line(...) et al, but it would just be a wrapper for constructing primitives:
    def draw_line(self, x1, y1, x2, y2):
        line = Line(x1, y1, x2, y2)
        self.root_context.add(line)
        return line

A DrawingContext would have the same wrappers - so you can draw on the canvas, or you can draw on a context; the difference being that a DrawingContext would act as a context manager, and any drawing operations on the DrawingContext would operate on the context, rather than the canvas directly.
The DrawingContext would also be a drawing instruction, whose role is to wrap other drawing instructions - that way a context can be put on the root context's list of drawing instructions.
So - usage would be something like this:
my_canvas = toga.Canvas()
my_canvas.draw_line(10, 20, 30, 40)
with my_canvas.context() as dc1:
    dc1.translate(5,15)
    dc1.draw_line(0, 0, 30, 40)
    with dc1.context() as dc2:
        dc2.draw_line(5, 10, 15, 20)

Looking at your use cases:

A simple drawing tool would just use the shortcut primitives to draw directly on the canvas.

A platform would construct a context for each character or element in the game, and retain handles to them. Moving the character is then just a matter of adjusting the context.

A vector tool would construct individual drawing objects, and add them explicitly to the canvas.

Does that make sense?

Thanks @freakboy3742 for the feedback! Here is version 2 based on my updated understanding.

@danyeaw Ok.. so... I've never found UML especially helpful as a communication tool (for Python especially) - but that diagram seems much further away from what got in my mind.
The drawing primitive objects are interface level objects - and even if they were at the level of the implementation, they wouldn't be "owned" in the way that diagram implies.
The role of the implementation is to provide a generic API that can map onto the native drawing APIs.
The role of the interface is to provide a mechanism to create and store a list of drawing instructions that, when needed, can be "executed", resulting in a bunch of implementation level drawing API calls (and, in turn, a bunch of native API calls)
Does that help any more? I feel like I'm not being especially effective in communicating what I've got in mind, but I'm not sure where the message is getting lost...

@freakboy3742. Thanks for your help getting the concept clear for me, I really appreciate it. I think what we really need is to be able to huddle around a whiteboard 😄

The calls to implementation layer drawing methods would be on the drawing objects.

Sorry, my fault, I think I originally read that and thought you meant that the drawing objects (classes) need to be on the implementation layer. Am I getting warmer?

@danyeaw That looks more like it, yes. The implementation layer doesn't need to have any concept of adding and removing drawing objects - the stack of drawing objects only needs to exist at the Interface layer. There might be some context handling operations on the implementation, but that's purely a GTK implementation detail - creating and destroying GTK drawing contexts (assuming that's the easiest way to render them).

@freakboy3742 Thanks! I have updated the diagram. I moved the stack to the interface layer and removed the add and remove drawing objects, you are right those shouldn't be needed on the implementation. I kept a draw_contexts method on the implementation because I think that will be needed for Gtk+, but might not be for other implementations.

Looks like it is time for me to start coding. Thanks again for all of your reviews and help!

@freakboy3742 I have finished initial implementation of the new design and the widget is drawing successfully using the updated Tutorial 4 example. To make this ready to merge I still have work to do on updating the tests. Would you mind taking a look?

Thanks so much for your time and effort taking another look at this @freakboy3742!

This may be a case where "documentation driven development" may help

I agree, so I will take that as an action to really polish the self documentation of the API. Before I tackle that though, I want to make sure what I have implemented is clear to both of us, and then we can look for areas where the current API is not what you expected.
First we have the different use cases:

The simple case of "draw a logo on the screen" is just a set of drawing instructions.

This is supported using the following API:
canvas = toga.Canvas(style=Pack(flex=1))
box = toga.Box(children=[canvas])
with canvas.fill()
    canvas.arc(50, 50, 15)

Notice that creation of a context is not needed. The with statement isn't to select a context, it just makes it easy to breakup what is going to be filled and stroked. It also adds niceties for other operations like closed_path where it creates a new path, allows you to draw on it, and then closes it for you. Getting rid of this feature would be very easy, but I think it is really nice for any type of complex drawing operation, otherwise you just have this huge flat list of operations and it is hard to tell what is being stroked and what is being filled.

A vector drawing app is a set of mouse click listeners to create objects, and drag listeners to modify the parameters of the object being displayed.

canvas = toga.Canvas(style=Pack(flex=1))
box = toga.Box(children=[canvas])
with canvas.fill()
    arc1 = canvas.arc(50, 50, 15)
    rect1 = canvas.rect(50, 50, 15, 15)

Then later you could say remove just the arc.
arc1.remove()

Maybe one thing currently missing, is I'm not sure that it is currently supported to do something like:
arc1(25, 25, 5)

A platformer game is a set of keyboard listeners to modify the x/y coordinate of a drawing context that draws each character on the canvas

canvas = toga.Canvas(style=Pack(flex=1))
box = toga.Box(children=[canvas])
context = canvas.create_context()
with context:
    with canvas.fill()
        canvas.arc(50, 50, 15)

Is there any way to return a context from a with statement? I would really prefer to have just one line, instead of one to create the context and one for the with statement. Contexts within contexts within contexts are currently supported.
Then later you could move that context on the screen:
context.translate(5, 5)

Which points out another hole in the implementation, it needs to recursively apply the translate operation to all of elements on the context (and on any sub-contexts).

If I'm reading this right, you're treating the canvas as a stateful object. You push and pop "context" onto the canvas, but always draw on the canvas itself.

All drawing objects are added to the canvas, but if you decide to use a context than those drawing objects are wrapped together in to a context group. You said early:

A DrawingContext would have the same wrappers - so you can draw on the canvas, or you can draw on a context; the difference being that a DrawingContext would act as a context manager, and any drawing operations on the DrawingContext would operate on the context, rather than the canvas directly.

I guess maybe what is different than what you were expecting is that you don't call it like this:
with context:
    with context.fill()
        context.arc(50, 50, 15)

If you go back and look at the UML drawing, there is both a Canvas class and a Context class. Should I have the Context class inherit from the Canvas class so that all the drawing operations are part of the Context?

This may be a case where "documentation driven development" may help
I agree, so I will take that as an action to really polish the self documentation of the API. Before I tackle that though, I want to make sure what I have implemented is clear to both of us, and then we can look for areas where the current API is not what you expected.

As an aside - the point of DDD is much like TDD - you do the docs first specifically because it avoids the need to develop (and re-develop) APIs over and over again while you iterate toward something that will work in practice.
I’ll leave it up to you whether this would be a helpful approach here.

First we have the different use cases:
The simple case of "draw a logo on the screen" is just a set of drawing instructions.
This is supported using the following API:
canvas = toga.Canvas(style=Pack(flex=1))
box = toga.Box(children=[canvas])
with canvas.fill()
canvas.arc(50, 50, 15)
Notice that creation of a context is not needed. The with statement isn't to select a context, it just makes it easy to breakup what is going to be filled and stroked.

Ok - that may be where the confusion is coming from. Part of the confusion is the there are a bunch of different types of object that are called (or we are calling) context.
There’s a Python language context, as created by a context manager. This is a language level construct, and a basis for creating an API.
Then there’s a Canvas context objects - the piece of the Javascript Canvas API that actually lets you draw. You can’t draw on a Javascript canvas itself; you can only draw on a “context” - with the end point being that you can have a different context (like a 3D context) and draw on that. This is an implementation detail, and shouldn’t be natively exposed to the user.
 They aren’t canvas contexts in the JS API, but they are “contextual”, and might be well served by being wrapped in a Python context.

It also adds niceties for other operations like closed_path where it creates a new path, allows you to draw on it, and then closes it for you. Getting rid of this feature would be very easy, but I think it is really nice for any type of complex drawing operation, otherwise you just have this huge flat list of operations and it is hard to tell what is being stroked and what is being filled.

Agreed - but this is where I see a benefit in Python level context objects being exposed - the Python context gives that “wrapper”, where the Javascript API only provides a flat list of operations.

A vector drawing app is a set of mouse click listeners to create objects, and drag listeners to modify the parameters of the object being displayed.
canvas = toga.Canvas(style=Pack(flex=1))
box = toga.Box(children=[canvas])
with canvas.fill()
arc1 = canvas.arc(50, 50, 15)
rect1 = canvas.rect(50, 50, 15, 15)
Then later you could say remove just the arc.
arc1.remove()
This would require the arc to know what container it belongs to (vs a container.remove(arc1) API, which doesn’t) - but yes, I agree that this is a desirable API.
Maybe one thing currently missing, is I'm not sure that it is currently supported to do something like:
arc1(25, 25, 5)
Is this intended as a “modify the existing arc1 object” API? If so… I’m not sure that’s a useful API; you’re using an “invocation” linguistic pattern, which doesn’t usually map to “in-place modification”. On the other hand, I would expect the following to work:

arc1.x_radius = 25
arc1.modify(y_radius=25, end_angle=5)

A platformer game is a set of keyboard listeners to modify the x/y coordinate of a drawing context that draws each character on the canvas
canvas = toga.Canvas(style=Pack(flex=1))
box = toga.Box(children=[canvas])
context = canvas.create_context()
with context:
with canvas.fill()
canvas.arc(50, 50, 15)
Is there any way to return a context from a with statement? I would really prefer to have just one line, instead of one to create the context and one for the with statement. Contexts within contexts within contexts are currently supported.

Yes.
with toga.context() as thing:
thing.stuff()
All you have to do is return the context object from the __enter__() method. Not sure what you have to do with @contextmanager (or if it is possible at all…)

Then later you could move that context on the screen:
context.translate(5, 5)
Which points out another hole in the implementation, it needs to recursively apply the translate operation to all of elements on the context (and on any sub-contexts).

Right - which is why drawing on a stateful canvas seemed odd.  If I draw on the canvas, and the context moves, I’m not sure the same expectation would exist.
It’s also something that is simplified if you use a tree/recursive approach to drawing, because the coordinate system is something that can be passed down through the tree walk - a parent draws it’s children at an origin; if the parent is “origin modifying”, that modification can be passed to all it’s children.

If I'm reading this right, you're treating the canvas as a stateful object. You push and pop "context" onto the canvas, but always draw on the canvas itself.
All drawing objects are added to the canvas, but if you decide to use a context than those drawing objects are wrapped together in to a context group.

Right - it’s the “wrapping as a group” that your current API doesn’t capture for me. Making the canvas track the “current” context means

I guess maybe what is different than what you were expecting is that you don't call it like this:
with context:
with context.fill()
context.arc(50, 50, 15)
Sort of; that’s more context managers than I was expecting. I would have expected something more like:

with canvas.fill() as filled_context:
    filled_context.arc(50, 50, 15)

If you go back and look at the UML drawing, there is both a Canvas class and a Context class. Should I have the Context class inherit from the Canvas class so that all the drawing operations are part of the Context?

I don’t know if that makes sense from a code perspective, but from logical/API point of view, yes - that’s what I’m driving at.
As an analog - Django’s ORM has the same methods on Manager and Query objects (anything you can do on Book.objects… you can also do on Book.objects.filter(..)…), but there isn’t a direct inheritance relationship; the Manager proxies it’s API onto a default query object.
(again, this is why UML fails as a documentation mechanism. It describes code structure, but doesn’t express logical structure)

This may be a case where "documentation driven development" may help
I agree, so I will take that as an action to really polish the self documentation of the API. Before I tackle that though, I want to make sure what I have implemented is clear to both of us, and then we can look for areas where the current API is not what you expected.

As an aside - the point of DDD is much like TDD - you do the docs first specifically because it avoids the need to develop (and re-develop) APIs over and over again while you iterate toward something that will work in practice.
I’ll leave it up to you whether this would be a helpful approach here.

First we have the different use cases:
The simple case of "draw a logo on the screen" is just a set of drawing instructions.
This is supported using the following API:
canvas = toga.Canvas(style=Pack(flex=1))
box = toga.Box(children=[canvas])
with canvas.fill()
canvas.arc(50, 50, 15)
Notice that creation of a context is not needed. The with statement isn't to select a context, it just makes it easy to breakup what is going to be filled and stroked.

Ok - that may be where the confusion is coming from. Part of the confusion is the there are a bunch of different types of object that are called (or we are calling) context.
There’s a Python language context, as created by a context manager. This is a language level construct, and a basis for creating an API.
Then there’s a Canvas context objects - the piece of the Javascript Canvas API that actually lets you draw. You can’t draw on a Javascript canvas itself; you can only draw on a “context” - with the end point being that you can have a different context (like a 3D context) and draw on that. This is an implementation detail, and shouldn’t be natively exposed to the user.
 They aren’t canvas contexts in the JS API, but they are “contextual”, and might be well served by being wrapped in a Python context.

It also adds niceties for other operations like closed_path where it creates a new path, allows you to draw on it, and then closes it for you. Getting rid of this feature would be very easy, but I think it is really nice for any type of complex drawing operation, otherwise you just have this huge flat list of operations and it is hard to tell what is being stroked and what is being filled.

Agreed - but this is where I see a benefit in Python level context objects being exposed - the Python context gives that “wrapper”, where the Javascript API only provides a flat list of operations.

A vector drawing app is a set of mouse click listeners to create objects, and drag listeners to modify the parameters of the object being displayed.
canvas = toga.Canvas(style=Pack(flex=1))
box = toga.Box(children=[canvas])
with canvas.fill()
arc1 = canvas.arc(50, 50, 15)
rect1 = canvas.rect(50, 50, 15, 15)
Then later you could say remove just the arc.
arc1.remove()
This would require the arc to know what container it belongs to (vs a container.remove(arc1) API, which doesn’t) - but yes, I agree that this is a desirable API.
Maybe one thing currently missing, is I'm not sure that it is currently supported to do something like:
arc1(25, 25, 5)
Is this intended as a “modify the existing arc1 object” API? If so… I’m not sure that’s a useful API; you’re using an “invocation” linguistic pattern, which doesn’t usually map to “in-place modification”. On the other hand, I would expect the following to work:

arc1.x_radius = 25
arc1.modify(y_radius=25, end_angle=5)

A platformer game is a set of keyboard listeners to modify the x/y coordinate of a drawing context that draws each character on the canvas
canvas = toga.Canvas(style=Pack(flex=1))
box = toga.Box(children=[canvas])
context = canvas.create_context()
with context:
with canvas.fill()
canvas.arc(50, 50, 15)
Is there any way to return a context from a with statement? I would really prefer to have just one line, instead of one to create the context and one for the with statement. Contexts within contexts within contexts are currently supported.

Yes.
with toga.context() as thing:
thing.stuff()
All you have to do is return the context object from the __enter__() method. Not sure what you have to do with @contextmanager (or if it is possible at all…)

Then later you could move that context on the screen:
context.translate(5, 5)
Which points out another hole in the implementation, it needs to recursively apply the translate operation to all of elements on the context (and on any sub-contexts).

Right - which is why drawing on a stateful canvas seemed odd.  If I draw on the canvas, and the context moves, I’m not sure the same expectation would exist.
It’s also something that is simplified if you use a tree/recursive approach to drawing, because the coordinate system is something that can be passed down through the tree walk - a parent draws it’s children at an origin; if the parent is “origin modifying”, that modification can be passed to all it’s children.

If I'm reading this right, you're treating the canvas as a stateful object. You push and pop "context" onto the canvas, but always draw on the canvas itself.
All drawing objects are added to the canvas, but if you decide to use a context than those drawing objects are wrapped together in to a context group.

Right - it’s the “wrapping as a group” that your current API doesn’t capture for me. Making the canvas track the “current” context means

I guess maybe what is different than what you were expecting is that you don't call it like this:
with context:
with context.fill()
context.arc(50, 50, 15)
Sort of; that’s more context managers than I was expecting. I would have expected something more like:

with canvas.fill() as filled_context:
    filled_context.arc(50, 50, 15)

If you go back and look at the UML drawing, there is both a Canvas class and a Context class. Should I have the Context class inherit from the Canvas class so that all the drawing operations are part of the Context?

I don’t know if that makes sense from a code perspective, but from logical/API point of view, yes - that’s what I’m driving at.
As an analog - Django’s ORM has the same methods on Manager and Query objects (anything you can do on Book.objects… you can also do on Book.objects.filter(..)…), but there isn’t a direct inheritance relationship; the Manager proxies it’s API onto a default query object.
(again, this is why UML fails as a documentation mechanism. It describes code structure, but doesn’t express logical structure)

Thanks @freakboy3742! :+1:
Here is my updated proposed API:
Simple
import toga
canvas = toga.Canvas(style=Pack(flex=1))
box = toga.Box(children=[canvas])
with canvas.fill() as fill:
    fill.arc(50, 50, 15)

Vector App
import toga
canvas = toga.Canvas(style=Pack(flex=1))
box = toga.Box(children=[canvas])
with canvas.fill() as fill:
    arc1 = fill.arc(50, 50, 15)
    rect1 = fill.rect(50, 50, 15, 15)

arc1.modify(25, 25, 5)
rect1.x = 75
fill.remove(rect1)

Platformer
import toga
canvas = toga.Canvas(style=Pack(flex=1))
box = toga.Box(children=[canvas])
with canvas.context() as hero:
    with hero.fill():
        hero.arc(50, 50, 15)
    with hero.stroke():
        hero.rect(50, 50, 15, 15)

hero.translate(10, 0)

I have updated the docs with this proposal. Once this is agreed to, then I will update the tests to match this proposal, and begin to work on final implementation of this API.

Those examples are exactly what I would have expected, right up until the "platformer" example - I would have expected something more like:
import toga
canvas = toga.Canvas(style=Pack(flex=1))
box = toga.Box(children=[canvas])
with canvas.context() as hero:
    with hero.fill() as body:
        body.arc(50, 50, 15)
    with hero.stroke() as outline:
        outline.rect(50, 50, 15, 15)

hero.translate(10, 0)

The difference being the 'targeting' of the arc() and rect() calls. I don't know if that was intentional or oversight; my reading of your version would have been and "unfilled arc" and an "unstroked rect" on the main context, rather than a "body arc" and "outline rect" on the hero context.
Just for clarification, I'd read that last example as:

The main "hero" context is both a Python and Javascript context. This would be the only explicitly created Javascript context in the example, but there would be an implicitly created context on the canvas itself (which is the context that the other two examples would be drawing on)

The body and outline create Python context, which are internally just a series of internal draw instructions, bracketed by the "begin" and "end" draw instructions for the fill/stroke.

Translating the "hero" context would implicitly move the arc and rect as well

If that matches what you're thinking, then I think we're good to go!

Thanks @freakboy3742 for the feedback! 👍
It was an intentional design. My thought was that users may not always care to store the context's yield value. For example I would definitely want to name the hero context so that I could move it later, but if I'm not planning on modifying the hero then I may not actually care what the name of the fill context is. In that case you could just call the context above it or the canvas if you want to conduct a draw operation using that context. I agree that it is more explicit and probably more clear if there is only one way and you have to always use the "as" keyword when creating a context. I'll update the docs with these changes.
Here is my idea for how this might be implemented. As you can probably tell by now, I'm a visual thinker 😄

I've got two concerns with that "implicit context" approach.
The first is the cognitive load on the user - they need to mentally manage an implicit context (the hero object) and an explicit context (the stroke/fill call). With the "implicit" approach, there are two different uses of context managers - one of which creates a context, and one that doesn't.
The second is one of implementation - if the context is explicit, then you don't need to manage any internal state on context objects - a context is just a list of drawing operations, one of which might be a context object itself. If it's implicit, then when a new Python context is created, the current Python context managing a Javascript context needs to be "pushed", so that any draw operations get added to the "implicit" current context...
While I'm sure it could be managed, it's a lot of complication for something that doesn't really gain you anything at the end of the day. Being explicit means it's always obvious what object the user is drawing on, and what context is currently in scope - and also makes it easy to draw on the "parent" context if some complex logic operation requires it.

@freakboy3742 In order to do a redraw after a new drawing object is added, I need the drawing object to call a redraw of the Canvas through to the implementation. The solution that has come to mind for this would be reuse the Tree widget for storing all of the drawing objects. I could then go from the current drawing object to the parent (and parent of parent, etc) until I find the Canvas to call a redraw on. An alternative would be to force the user to manually call a redraw via the API, but that doesn't sound like a nice solution. I haven't dug into the Tree and Tree Source too much besides a quick look, but do you agree that this would be a good approach to replace the current list of lists?

@danyeaw I can see similarities in the structure you need, but I'm not sure reuse is the best option here. There's enough tree-specific and canvas-specific logic involved that having a common subclass could cause more trouble than it's worth.
Personally, I'd suggest doing the same thing for context that we already do for app tracking on widgets. Widgets can have subwidgets; and every widget has an app. When you add a widget to an app, the widget iterates over all it's children and sets the app for it's children as well. That way, every widget can find it's app.
You only need half a dozen lines of code to implement this approach, and AFAICT it would fit reasonably nicely onto a base "drawing object" in terms of code structure.

@freakboy3742 Thanks for the guidance! :+1: I implemented a similar approach to the app/widget childs as a canvas/context childs and it is working well.
On my Gtk+ implementation it is currently just drawing everything to a single context using a callback. The last major thing I need to add is multiple contexts there so that, for example, the translate method moves everything in that context.

RFC: I am starting to think that the closed_path context adds extra complexity to the API and although it saves a call to new_path and close_path, I think it also makes things less explicit. Any time you would call closed_path, you would also need a stroke context. For example:
with canvas.stroke as stroker:
    stroker.closed_path(5, 5) as path_closer:
        path_closer.line_to(10, 10)
        path_closer.line_to(0, 0)

As an alternative we could have one less context:
with canvas.stroke as stroker:
    stroker.new_path(5, 5)
    stroker.line_to(10, 10)
    stroker.line_to(0, 0)
    stroker.close_path()

My goal is to push a ready for review version of the new stack implementation today. Any comments on which API is preferred?

@danyeaw My immediate response would be a hybrid of the two:
with canvas.closed_path(5, 5) as path_closer:
        path_closer.line_to(10, 10)
        path_closer.line_to(0, 0)

open/close style operations (where every open must be combined with a close) is the primary use case of a context, so unless there's a meaningful use case where you can start a path and not close it, that should be encompassed by a Python context.
The question then becomes - what's the purpose of the second (outer) context in your first example? Do you need to set up the stroke (which I assume is the pen width/color) independent of the path (the shape and fill color being described)? In that case, it makes a certain amount of sense that you could keep these two separate - consider the case of something like Tiberius' eyes - There are two distinct shapes (ok, they're circles, not line paths... but lets say he was wearing star-shaped rose-colored glasses 😀 ). In that case, both lenses will need the same line width and fill, but they're independent paths. In that case, I can see that there might be value in saying:
with canvas.stroke(...) as glasses_stroke:
    with glasses_stroke.closed_path(5,5) as left_lens:
        left_lens.line_to(...)
        ...
    with glasses_stroke.closed_path(5,5) as right_lens:
        right_lens.line_to(...)
        ...

However, as a shortcut for the common case - drawing a single shape with a given line style - being able to specify the stroke styling when creating the path would make sense to me.
Does that make any sense, or have I misunderstood what is going on here?

@freakboy3742 So if you want to draw something and not pickup your pen ("path") you don't need the closed_path at all, you just need to make sure that you eventually make it back to the starting place so that the path is "closed". You also don't need it if you are drawing a shape, only for lines. Your rose colored star glasses are the case where you would need to create a new path. Closing it with the close_path() method is actually optional, you could just use a final line_to your starting place, but it is convenient not needing to worry about where the starting place is.
Just because you say you want to close a path, doesn't mean you want to stroke (outline) it. You could also draw a closed path and then fill it with a color.
In my Tiberius example, the only thing I used the closed_path context for was to draw the outline of his head, since it was a combination of lines and arcs. Everything else was shapes (arcs, ellipses, and curves).

@freakboy3742 Thanks again for all of your great feedback and discussion so far.:+1: The API now supports the simple, vector app, and platformer use cases. It is ready for a review when you get the chance.

@freakboy3742 and I had a long review face-to-face. We agreed to make the following changes:

On the implementation layer, there will be only one native context supported for the whole canvas. Transformation operations (translate, scale, reset transform, etc) will be implemented on the interface layer, and will be applied to all children. This will reduce the complexity of the multiple context callbacks in Gtk+.
In the Gtk+ implementation, the context will be passed in to each drawing operation via optional kwargs.

So if we are going to put transformations on the interface layer, without completing redoing all drawing operations in Toga, I think there is going to be some limits on what transformations you can do.
These transformations are really designed to be done as the final step before drawing to the screen. For example, ideally you could create a rectangle and then translate, stretch, and skew it right before it gets drawn. Since we are doing the translations before the rectangle is created, you can't really skew a rectangle - because then it is, well, not a rectangle any longer.
You could I guess make the implementation layer just a bunch of vectors and then we could use these vectors and differential geometric calculations to draw everything, but then we would not be using each platform's native drawing capabilities.
Basic paths:

new_path with a line_to could support translation, stretch, and skew. This is a vector.
line_to from a path already being drawn needs to be restricted to the most limiting drawing operation. For example, if you did a line_to from an arc, if you can't skew the arc, you can't skew the line_to or what you draw would become distorted.

Basic shapes (curves, arc, rect):

Curves (bezier and quadratic) can support translation, stretch, and skew
Arc and ellipse can support translation for sure, and I think stretch based on modifying the radius (would have to think about the math on this one more). Skew is not supported.
Rect gets translate and stretch only.

Text:
Translate supported. Could maybe stretch by changing font size - but that would always change height and width.
So I would like to request feedback before I get any further in to coding this up. Since many of the operations wouldn't support skew, I would propose that we don't support skew at all. Is this an acceptable trade-off for everyone?

There's two possible interpretations of "transformation" here.
Firstly, there's a transformation you might want to do on an element-by-element basis on a canvas. Translations shouldn't be a problem to apply down the chain, but any other transformation (scale and skew) will absolutely be affected by the type of problem you've described.
In the first pass, I'd be completely OK if we just punted on scale/skew operations at this level of the API. Yes, it would be nice to have them, but it won't be easy to implement them, so if we have to leave them out, I don't think that will matter too much. Looking at the use cases we've been tracking for API design, simple graphics display won't be using transforms; a platformer really only needs translate to move the character around; and a vector graphics tool is going to be altering object parameters directly (so a GUI operation to "scale rectangle 2x" would be manually modifying the width/height parameters directly, rather than introducing a scale operation).
The second is a canvas-level transformation. This is an operation that is defined by the canvas itself, so it doesn't matter what you draw, the canvas will be able to compensate. At this level, "scale" is effectively "zoom", so this is a bunch of functionality that I can see would be helpful to retain. Another interpretation would be the analog with images - having set an image size, do you scale the image to fit the view, or centre the image at a known location, etc.
Does that make sense?

@freakboy3742 makes sense to me, thanks for your time responding! :gift:
I might slightly alter your description of the first type of transformation from element-by-element to apply to all elements on python context. We'll punt on that one for now.
I will implement the 2nd type, canvas transformation, which will apply to the whole canvas no matter which python context it is created with.

@freakboy3742 Thanks so much for all the time you spent on this so far. :+1: I put some more :heart: in to this PR and I am ready for another review.

Thanks again for all your comments and time reviewing :+1:
After taking another look at the children_contexts list, I determined that holding all the contexts in a tree wasn't required in order to pass the canvas down to the next context. The updated PR now just passes the canvas down when a context is created. I removed the children_contexts list, so thanks for helping me find an area that could be simplified. :champagne:
I fixed and updated the majority of your other comments.

For optimizing the redraw method, I recommend tackling that as part of #319.
For moving native_context to the first argument, I'm not sure that is feasible. I am calling the implementation methods from the interface with impl.drawing_object(self, ...), so the impl gets passed in as the self.
I think the modify methods are also needed as I explained above. But, please let me know if you think I'm missing something.
I am also not sure how to make the contexts draw themselves also explained more above. This is a limitation of needing to call the drawing objects from the implementation layer. If you think this is possible, would you mind please explaining more about your concept?

On reflection, the redraw has a natural fix - the enter and exit methods of the python context manager provide a point at which we can suspend and resume updates. It shouldn't be that difficult to achieve.
When I said "first argument", I meant the first one after self :-)
I've left a comment about modify above
The interface contexts don't need to draw themselves; the contexts need to know how to draw on a GTK context when provided one.

The impl layer widget has a member method:

def gtk_draw_callback(self, gtk_context):
    self.interface.root(self, draw_context=gtk_context)

This callback method is registered as self.native.connect("draw", self.gtk_draw_callback)
The "root" object on the interface widget is a drawing object object only job is to be a container for other drawing objects. It might also be the only drawing object that exposes the canvas-level scale/skew options. It has a __call__ method that is essentially:

def __call__(self, impl, draw_context):
    for obj in self.drawing_objects:
        obj(impl, draw_context)

A "simple" drawing object (something like line_to) has a __call__ method that is something like:

def __call__(self, impl, draw_context):
    impl.line_to(draw_context, self.x, self.y)

A more complex drawing object (something like a path) is a combination of the two styles, wrapping the "contained" drawing objects with the open/close pair of calls on the implementation that the python context represents.

def __call__(self, impl, draw_context):
    impl.start_path(draw_context, self.x, self.y)
    for obj in self.drawing_objects:
        obj(impl, **kwargs)
    impl.end_path(draw_context)

(the exact calls in this last example aren't quite right for Path, but hopefully it's sufficiently clear what I'm aiming at)
This also possibly negates my argument about the ordering of the context argument in draw operations, because it might make sense to pass around **kwargs at the interface level to decouple the specific drawing context arguments that a backend will require.
Does that make any more sense?

@freakboy3742 I think I understand what you described, thanks for the examples!

I changed the redraw events so that they only happen when exiting a context or when drawing directly to the canvas. This reduced redraw events for the Tiberius example from 55 to 12.
I did not reorder the contexts arguments based on you saying your argument was negated by the more complex drawing object above.
I removed all the modify methods, all changes are now done directly to the object properties.
I implemented the design based on your examples above. One thing I did change is that I realized after I implemented the root drawing object, that I could directly call the Canvas instead. This simplified the design further, while still maintaining clarity.

Thanks again for all of your great feedback.

Thanks again for all your great feedback!

I moved the transformation methods to the Canvas class.
I understand your concept of propagating an origin offset, but if you don't mind I would prefer to open up a separate PR for this once I detail out how I want the API to look.
I replaced the call methods with new draw methods.


Frontend not starting at develop branch

term-project-2024-team-4-backend-1   | Default catalogue initialization successful.
term-project-2024-team-4-frontend-1  | 6:03:40 PM [vite] vite.config.js changed, restarting server...
term-project-2024-team-4-frontend-1  | node:internal/process/promises:288
term-project-2024-team-4-frontend-1  |             triggerUncaughtException(err, true /* fromPromise */);
term-project-2024-team-4-frontend-1  |             ^
term-project-2024-team-4-frontend-1  |
term-project-2024-team-4-frontend-1  | Error: EIO: i/o error, stat '/frontend/src'
term-project-2024-team-4-frontend-1  | Emitted 'error' event on FSWatcher instance at:
term-project-2024-team-4-frontend-1  |     at FSWatcher._handleError (file:///frontend/node_modules/vite/dist/node/chunks/dep-9A4-l-43.js:47909:10)
term-project-2024-team-4-frontend-1  |     at NodeFsHandler._addToNodeFs (file:///frontend/node_modules/vite/dist/node/chunks/dep-9A4-l-43.js:46726:18)
term-project-2024-team-4-frontend-1  |     at async file:///frontend/node_modules/vite/dist/node/chunks/dep-9A4-l-43.js:47713:21
term-project-2024-team-4-frontend-1  |     at async Promise.all (index 0) {
term-project-2024-team-4-frontend-1  |   errno: -5,
term-project-2024-team-4-frontend-1  |   code: 'EIO',
term-project-2024-team-4-frontend-1  |   syscall: 'stat',
term-project-2024-team-4-frontend-1  |   path: '/frontend/src'
term-project-2024-team-4-frontend-1  | }
term-project-2024-team-4-frontend-1  |
term-project-2024-team-4-frontend-1  | Node.js v18.19.1
term-project-2024-team-4-frontend-1 exited with code 1
there were 3 out of 4 containers working

It looks like this has something to do with the node_modules. Please refer to issue #91

it was working with the given solution


Replace several redirects with a single overwrite

Some redirection is either missing or excessive resulting in a fact that portal is not generated in Neither at height 1000.
I tried to redirect MathHelper.clamp(...) but apparently this is not enough to make it work. Here is lazy but ironconcrete solution.

While you are modifying it, can you also add isCubicWorld checks?

Done.


[Question] Utilizing cameraTextureName to process image data

Hi there! So I'm aware that you can grab the camera texture name (id) from the ViroView. However I have't been able to find a good answer as to how to utilise the id. What is the best way to get the id into Bitmap form so that I can process it? Thanks in advance.

Hi @Alex-Lai,
Use this TextureReader.java class provided by Google for grabbing the texture info.
That'll give you the data as a TextureReaderImage.java which is just a wrapper around a ByteBuffer.
You can then use this data as-is, or you could convert it into a BitMap (not advised since the ByteBuffer itself already contains the data in its raw form). Here's on example on StackOverflow regarding converting a ByteBuffer -> Bitmap.
Let us know if that works!

Hi @Alex-Lai, in our next release there will be a more efficient way to to do this by way of a listener interface which will give you the raw image data each frame for you to process.

Thanks @achuvm for the info, I'll give that a try. @radvani sounds promising, looks forward to giving it a try! Is there an estimate on the date of the next release?

Yes, this next release should be out early next week. I would recommend waiting for that for processing image data as the previous camera texture ID method is quite difficult to get working correctly.

CameraImageListener added to latest release, ViroCore 1.9
Release notes and upgrade instructions here -> https://virocore.viromedia.com/docs/releases


new docs front page

Overview
We need to make changes to Botkube's docs front page to be more user-friendly and introduce a new section about AI assistant. Notion page.
Format should be similar to https://docs.testkube.io/
Specifically:
De-emphasize open-source focus: Revise the installation page to downplay open-source aspects and make cloud the default installation.
Add AI section: Create a dedicated AI section on the usage page(how it is pre-installed, how to disable it, generic image of what is/architecture, how to get started with 3-5 examples commands)
Acceptance Criteria

[ ] Introduce "Home" Page, Dashboard which navigate user to the most important parts of the docs. This page also should bold the main features of Botkube.
[ ] Update Installation Documentation: The revised installation page should reduce its emphasis on being open-source.
[ ] Create AI Section in Usage Documentation: The usage page should include a new AI section with the elements described above.

Reason
Improved User Experience (Installation)
Enhanced Understanding of AI Assistant

@miklosp we need your input here. Designs of the home page at least


Combine multiple PDFs bug?

Hi, I'm trying to combine multiple similar PDFs.
All 4 input files are nearly identical, except for a text with some unique number.
All PDFs have a single page.
The output PDF has 4 pages, yet all pages are identical.
const PDFLib = require("pdf-lib");

/**
 * Combines multiple PDFs into one.
 *
 * @param {Buffer[]} arrBuffers
 */
async function combine(arrBuffers)
{
	if(!arrBuffers.length)
	{
		throw new Error("arrBuffers is empty. Need at least one PDF to do something right.");
	}

	const finalPDF = PDFLib.PDFDocumentFactory.load(arrBuffers.shift());

	while(arrBuffers.length)
	{
		const extraPDF = PDFLib.PDFDocumentFactory.load(arrBuffers.shift());

		for(const page of extraPDF.getPages())
		{
			finalPDF.addPage(page);
		}
	}

	return await PDFLib.PDFDocumentWriter.saveToBytes(finalPDF);
};

Hello @oxygen. I took a look at the code for page addition. It seems that while the page object itself is being copied into the document, its content streams and resource objects are not being copied. The PDFDocument.addPage and PDFDocument.insertPage methods will need to be updated to copy these additional objects if the page being added comes from a different document.

@oxygen I just published a new pre-release: 0.3.1-rc3. It contains a fix for this issue. Please try it out when you get a chance and let me know if it works for you.
You can install this pre-release with npm:
npm install<EMAIL_ADDRESS>
It's also available on unpkg:

<EMAIL_ADDRESS><EMAIL_ADDRESS>
Note that I'm still working on refining the the page copying logic to handle some edge cases. Stuff like adding the same page to the destination document multiple times, or modifying the page in the source document after adding it to the destination document. But the version contained in this pre-release should work just fine for your use case (assuming it isn't more complicated than the example you shared).
(Here's the branch for this fix, if you're interested: https://github.com/Hopding)/pdf-lib/tree/CopyPages.

Version 0.5.0 is now published. It contains a completed implementation of the page copying between documents feature. The full release notes are available here.
You can install this new version with npm:
npm install<EMAIL_ADDRESS>
It's also available on unpkg:

<EMAIL_ADDRESS><EMAIL_ADDRESS>


Error handling with multiple concurrent connections

@meejah as per our discussion on slack; if you have a WAMP client that runs multiple concurrent connections via the Component interface, and one of those connections errors, AND in the event you have retries turned off, an exception makes it's way back up the chain causing 'run' to exit. So if you are for example connecting to three different Crossbar's, and ONE of the connections to one of the Crossbar's fails, the entire app will fail. Sample code;
#!/usr/bin/env python3

import sys
from autobahn.asyncio.component import Component
from autobahn.asyncio.component import run
from autobahn.wamp.types import SubscribeOptions
import txaio
txaio.use_asyncio()
#
log = txaio.make_logger()
if sys.argv[-1] == '--debug':
    sys.argv.pop()
    txaio.start_logging(level='debug')
else:
    txaio.start_logging(level='info')
#
#
routers = []
router_definitions = [
    [
        {'name': 'instance1', 'url': 'ws://localhost:8091/ws', 'realm': 'realm1'},
        {'name': 'instance2', 'url': 'ws://localhost:8092/ws', 'realm': 'realm1'},
        {'name': 'instance3', 'url': 'ws://localhost:8093/ws', 'realm': 'realm1'},
    ]
]

class RouterComponent(Component):

    def __init__(self, router):

        self._router = router
        self._session = None
        transports = {
            'url': router['url'],
            'max_retries': 0
        }
        super().__init__(
            transports=[transports],
            realm=router['realm']
        )

    def startup(self):
        self.on_join(self.on_connection_join)
        self.on_leave(self.on_connection_leave)
        return self

    @property
    def name(self):
        return self._router['name']

    async def on_connection_join(self, session, details):

        self._session = session
        options = SubscribeOptions(match='exact', details=True)
        await session.subscribe(self.subscription_on_create, 'wamp.subscription.on_create', options)
        await session.subscribe(self.subscription_on_subscribe, 'wamp.subscription.on_subscribe', options)
        await session.subscribe(self.subscription_on_unsubscribe, 'wamp.subscription.on_unsubscribe', options)
        await session.subscribe(self.subscription_on_delete, 'wamp.subscription.on_delete', options)

    async def on_connection_leave(self, session, details):
        log.info(f'on_connection_leave: [{self.name}]: {details}')

    async def subscription_on_data(self, data, details=None):
        log.info(f'subscription_on_data [{self.name}]: {data} :: {details.topic}')

    async def subscription_on_create(self, session_id, sub_details, details=None):
        log.debug(f'subscription_on_create [{self.name}]: session_id={session_id} :: id={sub_details["id"]} uri={sub_details["uri"]}')

    async def subscription_on_subscribe(self, session_id, sub_id, details):
        log.debug(f'subscription_on_subscribe [{self.name}]: session={session_id} :: sub_id={sub_id}')

    async def subscription_on_unsubscribe(self, session_id, sub_id, details):
        log.debug(f'subscription_on_unsubscribe [{self.name}]: session={session_id} :: sub_id={sub_id}')

    async def subscription_on_delete(self, session_id, sub_id, details=None):
        log.debug(f'subscription_on_delete: session={session_id} :: sub_id={sub_id}')

if __name__ == "__main__":

    for router in router_definitions[0]:
        component = RouterComponent(router).startup()
        routers.append(component)

    run(routers)

Sample output, run application, then Ctrl+C in one of the crossbar consoles;
$ ./substest.py
2018-11-07T11:04:15 connecting once using transport type "websocket" over endpoint "tcp"
2018-11-07T11:04:15 connecting once using transport type "websocket" over endpoint "tcp"
2018-11-07T11:04:15 connecting once using transport type "websocket" over endpoint "tcp"
2018-11-07T11:04:18 session leaving 'wamp.close.transport_lost'
2018-11-07T11:04:18 on_connection_leave: [instance2]: CloseDetails(reason=<wamp.close.transport_lost>, message='WAMP transport was lost without closing the session before')
2018-11-07T11:04:18 Connection failed: TransportLost: failed to complete connection
2018-11-07T11:04:18 Component failed: Exhausted all transport connect attempts
2018-11-07T11:04:18 Component '<__main__.RouterComponent object at 0x7f51278eab00>' error: RuntimeError: Component failed: Exhausted all transport connect attempts

Specific use-case
I want to be able to connect to a dynamic list of Crossbar servers, in the instance one should fail, I want to remove it from the list and potentially add a new one. Currently I don't get the chance because the entire app exits on an exception.
... I think the exception stems from wamp/component.py here;
    dl = []
    for comp in components:
        d = component_start(comp)
        dl.append(d)
    done_d = txaio.gather(dl, consume_exceptions=False)

    def all_done(arg):
        log.debug("All components ended; stopping reactor")
        done_callback(reactor, arg)

i.e. the consume_exceptions = False

Interesting, never anticipated that Component would be sub-classed ;)
This might be a case where you'd need your own "run"-type method; the semantics for "run" are basically: keep all these Components running, and if any of them fail outright, then fail. So in the above, one of your components is failing ("wamp.close.transport_lost").
Have you already tried the is_fatal suggestion? (e.g. add an is_fatal handler that says nothing is fatal)

Also, why are you starting logging yourself but also using run..?

yeah, agreed. I mean, not subclassing .. was a major reason for Component in the first place.

@oberstet I think this might be for monitoring multiple routers? (But also, yeah: why not just re-connect to them .. or let the component keep trying to re-connect, anyway)

So, I wasn't even thinking "HA stuff" here, just wondering why you'd want to have a component "just stop running" while other ones do -- but I guess there's going to be use-cases for that, so ..

For Crossbar OS users for example, if you want to monitor your systems, and you've split your environment using a number of Crossbars, you may well want to connect to all of your crossbars for stats and uptime .. in this context, crossbars will reboot (or crash, or be upgraded) and you may want to add or remove environments .. in these instances, you probably don't want your monitor to terminate ... :) .. or you may want to notify a process running in one environment of an event in another environment, so you may want your own simple notification bridge that may need to connect to one or more crossbars .. same applies ... generically, whereas the 'terminate when all components have finished' makes complete sense to me, 'terminate when one component suffers a disconnect' .. less so ..

Well, okay that's a fine use-case -- but why not just handle join and leave events? That is, your component keeps "running" (i.e.trying to re-connect) to the now-down crossbar instance. If you just drop a component out of the run list, how would it ever get back into that list?
I guess what I'm getting at here is: the right way to learn if your Component is connected or not is via the leave/join events. The only use-case I see for max_retries=0 is if you're writing a quick one-off client that you want to connect ONCE only, but see an immediate failure if the connection fails (so, e.g. maybe some kind of "status" CLI command or similar).

Ok, so for the monitor example, if I have a new Crossbar to monitor, would I not want to add a component to the array of active components, rather than restarting the app? (and the reverse if a crossbar instance becomes obsolete or fails permanently?)

Managing multiple connections is outside the scope of the component API - the component API is about this use: run multiple app components on over one WAMP connection.

@oberstet well, no it can run mulitple WAMP connections too .. but yes the API never really anticipated trying to add and remove components themselves.
@oddjobz maybe, but that's not what your code is doing; it's just failing a component right away as soon as the connection dies (once).  (In general, it's pretty hard to get "dynamically re-load the configuration on-the-fly" correct .. I'm not suggesting that would be a bad thing to have, but definitely it's outside the scope of run).
If you're not really using the re-connect logic Component provides, and not using the promises of run this is really where you should just use .start() on Component directly and have your own main-loop (i.e. "don't use run"). At a bare minimum, run would need more features to support "add and remove components whenever". But it would likely make more sense to have a completely separate API for this (e.g. a Service in Twisted) and keep run simple and focused.
This would be like a kind of "dynamically manage Components" API. That said, though, any "monitoring" application like this would have to be able to re-start. Once you have that feature, what's wrong with just using it to re-configure the list of expected "things to monitor"..?

In this vein, an API addition I think might make sense to Component: adding an async def session() which will: insist that max_retries=0 (i.e. no reconnects) and fires with a autobahn.wamp.Session instance once connected (or an error). The use-case for this would be one-off clients that want a single WAMP session or an error, nothing else. So then you'd do something like:
my_component = Component(...)

async def main(reactor):
    session = await my_component.session()
    await session.call(...)
    # etc...
    await session.leave()
react(lambda r: ensureDeferred(main(r)))

For example, you want to connect to a WAMP router, make a few call()s and then leave. Probably for CLI tools, or similar.

@meejah

maybe, but that's not what your code is doing;

No, it's just a snippet for testing, bit it's what the code would like to do .. :)
Ok, this change to component.py modifies the exit behaviour, i.e. so it doesn't;
    done_d = txaio.gather(dl, consume_exceptions=True)

I'll have a think about a run() when I come back to what I was testing. In the meantime I hear what you're saying about restarting being the easy path, and it may well be a preferred option for many use-cases. That said for applications in general, having to restart the entire application just to reconfigure it, is not always what users want. If for example we had to restart the UI when someone added a new node, it wouldn't really qualify as a modern reactive SPA .. there might even be a few comments on google groups indicating that people might like to be able to reconfigure Crossbar without having to restart it ... ;)
I tweaked my example a little, just as a coding exercise .. is this what you meant by composition rather then inheritance .. or am I still missing something?
#!/usr/bin/env python3

from autobahn.asyncio.component import Component
from autobahn.asyncio.component import run
from autobahn.wamp.types import SubscribeOptions
import txaio
txaio.use_asyncio()
log = txaio.make_logger()

class Crossbars:

    class Crossbar:

        def __init__(self, name, port):
            self._name = name
            self._port = port

        @property
        def name(self):
            return self._name

        @property
        def port(self):
            return self._port

        @property
        def url(self):
            return f'ws://localhost:{self._port}/ws'

        @property
        def realm(self):
            return 'realm1'

    def __init__(self):
        self._instances = []

    def add(self, name, port):
        self._instances.append(self.Crossbar(name, port))

    def __iter__(self):
        return iter(self._instances)

class RouterComponent:

    def __init__(self, crossbar):
        self._crossbar = crossbar
        self._session = None
        transport = {'url': crossbar.url, 'max_retries': 0}
        self._component = Component(transports=[transport], realm=crossbar.realm)
        self._component.on_join(self.on_connection_join)
        self._component.on_leave(self.on_connection_leave)

    @property
    def component(self):
        return self._component

    @property
    def name(self):
        return self._crossbar.name

    async def on_connection_join(self, session, details):
        self._session = session
        options = SubscribeOptions(match='exact', details=True)
        await session.subscribe(self.subscription_on_create, 'wamp.subscription.on_create', options)
        await session.subscribe(self.subscription_on_subscribe, 'wamp.subscription.on_subscribe', options)
        await session.subscribe(self.subscription_on_unsubscribe, 'wamp.subscription.on_unsubscribe', options)
        await session.subscribe(self.subscription_on_delete, 'wamp.subscription.on_delete', options)

    async def on_connection_leave(self, session, details):
        log.info(f'on_connection_leave: [{self.name}]: {details}')

    async def subscription_on_data(self, data, details=None):
        log.info(f'subscription_on_data [{self.name}]: {data} :: {details.topic}')

    async def subscription_on_create(self, session_id, sub_details, details=None):
        log.debug(f'subscription_on_create [{self.name}]: session_id={session_id} :: id={sub_details["id"]} uri={sub_details["uri"]}')

    async def subscription_on_subscribe(self, session_id, sub_id, details):
        log.debug(f'subscription_on_subscribe [{self.name}]: session={session_id} :: sub_id={sub_id}')

    async def subscription_on_unsubscribe(self, session_id, sub_id, details):
        log.debug(f'subscription_on_unsubscribe [{self.name}]: session={session_id} :: sub_id={sub_id}')

    async def subscription_on_delete(self, session_id, sub_id, details=None):
        log.debug(f'subscription_on_delete: session={session_id} :: sub_id={sub_id}')

if __name__ == "__main__":

    crossbars = Crossbars()
    crossbars.add('instance1', 8091)
    crossbars.add('instance2', 8092)
    crossbars.add('instance3', 8093)

    routers = []
    for crossbar in crossbars:
        routers.append(RouterComponent(crossbar).component)

    run(routers)

Sorry for the delay. Something more like this, @oddjobz:
#!/usr/bin/env python3

import sys
from autobahn.asyncio.component import Component
from autobahn.asyncio.component import run
from autobahn.wamp.types import SubscribeOptions

urls = [
    'ws://localhost:8091/ws',
    'ws://localhost:8092/ws',
    'ws://localhost:8093/ws',
]

async def subscription_on_data(name, data, details=None):
    log.info(f'subscription_on_data [{name}]: {data} :: {details.topic}')

async def subscription_on_create(name, session_id, sub_details, details=None):
    log.debug(f'subscription_on_create [{name}]: session_id={session_id} :: id={sub_details["id"]} uri={sub_details["uri"]}')

async def subscription_on_subscribe(name, session_id, sub_id, details):
    log.debug(f'subscription_on_subscribe [{name}]: session={session_id} :: sub_id={sub_id}')

async def subscription_on_unsubscribe(name, session_id, sub_id, details):
    log.debug(f'subscription_on_unsubscribe [{name}]: session={session_id} :: sub_id={sub_id}')

async def subscription_on_delete(name, session_id, sub_id, details=None):
    log.debug(f'subscription_on_delete: session={session_id} :: sub_id={sub_id}')

components = []
for idx, url in enumerate(urls):
    comp = Component(
        transports=[
            "url": url,
            "max_retries": 0,
        ],
        realm="realm1",  # obviously, could come from 'urls' list or so instead
    )
    name = "router_{}".format(idx)

    # instead of "name" we could make a data-class style
    # MonitoredRouter or similar and bind the functions below to use
    # that instead (if, e.g., more than just "the name" is important)

    @comp.on_join
    def setup(session, details)
        options = SubscribeOptions(match='exact', details=True)
        await session.subscribe(
            partial(subscription_on_create, name)
            'wamp.subscription.on_create',
            options,
        )
        await session.subscribe(
            partial(subscription_on_subscribe, name),
            'wamp.subscription.on_subscribe',
            options,
        )
        await session.subscribe(
            partial(subscription_on_unsubscribe, name),
            'wamp.subscription.on_unsubscribe',
            options,
        )
        await session.subscribe(
            partial(subscription_on_delete, name),
            'wamp.subscription.on_delete',
            options,
        )

    @comp.on_leave
    def left(session, reason):
        log.info(f'on_connection_leave: {reason}')

    components.append(comp)

if __name__ == "__main__":
    run(components)

This is: simple, easy-to-test functions that are tied together by calling other functions, no complex classes and no subclassing of anything in Autobahn.


Add migration guide

Add migration guide for changes from #47

It looks we preserve public entities structures like translators and tts, so migration is not required

Anyway, look at https://github.com/translate-tools/linguist/pull/387 to find a changes that required to update version


Parallel_runner and episode_runner show obvious reward difference for the same test_battle_won_mean

Hi, thank you for your amazing contribution! I'm doing some research based on qmix which may include parallel and episode runner at the same training stage. But I got reward around 18 when test_battle_won_mean reached 85%  for parallel_runner, while episode_runner only produced reward around 11 for the similar test_battle_won_mean on map MMM2. Can you tell me the crucial difference between the 2 runner that produce different reward? By the way, it seems that parallel_runner's 8*sample number performs worse when trained for 2 million steps on MMM2, could you please shed some light on this? Thanks a lot!

It seems that I used an older version of smac which may calculate the enemy health twice for max reward  once the first attempt to init_unit failed. When upgraded to the new smac, the problem disappeared.


fix: don't display 'Collection is empty' label if collection has any …

…requests

Hey @vlad0337187,
Your changes look good to me!
View build log
TravisBuddy Request Identifier: 7772c640-f5bf-11e9-a1fe-396a3052531f